[PATCH] GetBrightness method has to return _unsigned_ int

Holger Macht hmacht at suse.de
Wed May 30 07:01:03 PDT 2007


On Mi 30. Mai - 14:47:09, Richard Hughes wrote:
> On Wed, 2007-05-30 at 14:20 +0200, Holger Macht wrote:
> > The GetBrightness method in the macbook* addons wrongly supplies a signed
> > integer as the first argument in the reply. This makes a call similar to
> > 
> >   dbus_message_get_args(reply, &err, DBUS_TYPE_UINT32,
> > 	                &brightness, DBUS_TYPE_INVALID)) {
> > 
> > fail in the calling process.
> 
> Yes I failed to find the bug a few months ago where the dell addon won't
> work with g-p-m.

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 ;-)

> 
> > However, all applications I looked into (gpm, kpowersave, powersaved) have
> > a call similar to this.
> > 
> > I currently don't see where it is specified that the GetBrightness method
> > has to return an UINT32 at all. Neither the
> > hal-system-lcd-get-brightness-linux script nor
> > 10-laptop-panel-mgmt-policy.fdi doesn't assume this AFAICS. Maybe someone
> > can tell...
> 
> See http://bugzilla.gnome.org/show_bug.cgi?id=404474 - there's lots of
> data there.

Ok, I see.

Regards,
	Holger


More information about the hal mailing list