[PATCH] fix mapping of chassitype to system.formfactor

David Zeuthen david at fubar.dk
Wed May 10 11:04:10 PDT 2006


On Wed, 2006-05-10 at 18:25 +0200, Danny Kukawka wrote:
> The current code work correct, but I take a look at this (but this blow up the 
> code).

No, the current code is not correct. We really need to do this when
synthesizing the event because we need to set system.formfactor on the
computer object before it's added to the GDL. Thanks for looking into
this.

> Yes, I know this problem in the 'old' code. But I would also avoid do all the 
> mapping (init the map, do check against smbios.chassis.type) if there was 
> already set system.formfactor in a other place. 
>
> Because of this, I removed set system.formfactor='unknown' in general (in 
> osspec_probe() ) and set it only if execute the prober failed (or 
> should_decode_dmi is FALSE) or if the property isn't set already. But for 
> this I need the lock with device_property_atomic_update_*().

No, I don't think you need that. Btw, property_atomic_update_*() only
ensures that updating multiple properties are sent out as a single dbus
PropertyChanged changed. It doesn't really "lock" anything, it just
queues up changes for the dbus interface. Which is kinda useless for
coldplug as we haven't even claimed the org.freedesktop.Hal service
before all coldplug is done (to prevent sending out lots of signals when
adding devices in coldplug).

> > What is the original problem that the patch was trying to address? Does
> > my proposed fix above fix it?
> 
> Maybe, but see above ... this is not needed if we check if the key is already 
> set. IMO you need to check this also again before set the property if you 
> have a match and also a lock around this to be 100% sure that you don't 
> overwrite some values, or not? Is this lock such a problem?

It's still not a lock and I'm just not sure why need a lock at all?
Everything runs in a single thread and we are 100% guaranteed that no
other code (either async on mainloop or out-of-process) runs when we are
in computer_probing_pcbios_helper_done().

I think the problem you were seeing was related to the missing "== 0" in
the strcmp.

    David




More information about the hal mailing list