[PATCH] GetBrightness method has to return _unsigned_ int

[David Zeuthen]

Hi,

On Wed, 2007-05-30 at 16:01 +0200, Holger Macht wrote:
> After digging a bit more, it's actually not dbus_message_get_args(...)
> which fails. It does some internal cast so that it both works with INT32
> and UINT32. The real problems actually arise as soon as one explicitly
> checks the return type. So at least HAL has to be consistent which is what
> my patch actually achieves. It misses the needed omap backlight fixes,
> however. Let's just wait David says about this, because there are two
> possibilities IMHO: Fix all addons and document that GetBrightness returns
> _unsigned_ int, or fix HAL to not cast the exit status from scripts to
> _unsigned_ int ;-)

The spec says (in it's own way) we return an INT32

http://people.freedesktop.org/~david/hal-spec/hal-spec.html#device-properties-laptop-panel

and all methods implemented via spawning a helper _always_ return INT32
(there's a TODO item for 0.5.10 about returning other types)

http://people.freedesktop.org/~david/hal-spec/hal-spec.html#device-properties-info-method-calls

So it's like this

 - applications such as g-p-m, kpowersave should expect INT32

 - Introspection XML returned by HAL needs to say it's INT32

 - HAL should return INT32 over the wire; except for method calls
   using spawned helpers. The culprit is in hald_exec_method_cb()
   in hald/hald_dbus.c. I've committed this patch

> --- a/hald/hald_dbus.c
> +++ b/hald/hald_dbus.c
> @@ -3675,7 +3675,7 @@ hald_exec_method_cb (HalDevice *d, guint32 exit_type,
>                      gint return_code, gchar **error,
>                      gpointer data1, gpointer data2)
>  {
> -       dbus_uint32_t result;
> +       dbus_int32_t result;
>         DBusMessage *reply = NULL;
>         DBusMessage *message;
>         DBusMessageIter iter;
> @@ -3720,14 +3720,14 @@ hald_exec_method_cb (HalDevice *d, guint32 exit_type,
>                 dbus_message_unref (reply);
>  
>         } else {
> -               result = (dbus_uint32_t) return_code;
> +               result = (dbus_int32_t) return_code;
>  
>                 reply = dbus_message_new_method_return (message);
>                 if (reply == NULL)
>                         DIE (("No memory"));
>                 
>                 dbus_message_iter_init_append (reply, &iter);
> -               dbus_message_iter_append_basic (&iter, DBUS_TYPE_UINT32, &result
> +               dbus_message_iter_append_basic (&iter, DBUS_TYPE_INT32, &result)
>                 
>                 if (conn != NULL) {
>                         if (!dbus_connection_send (conn, reply, NULL))

I wonder what will break from this bugfix...

     David