[PATCH] fix mapping of chassitype to system.formfactor

[Danny Kukawka]

On Wednesday 10 May 2006 17:34, David Zeuthen wrote:
[...]

>    (notably hald/linux2/apm.c is wrong; it sets system.formfactor in
>     the wrong place; e.g. needs to be set while synthesizing the event;
>     that needs to be fixed)

The current code work correct, but I take a look at this (but this blow up the 
code).

[...]
> As far as I can see, at worst computer_probing_pcbios_helper_done()
> needs to get some more smarts so as to not overwrite system.formfactor
> in the event system.formfactor is already 'laptop' because one of the
> ACPI/PMU/APM hotplug synthesizers set it that way.
>
> The code itself looks broken
[...]
> The problem here is that we only set system.formfactor if it's not
> "unknown" already. Someone likely forgot the "== 0" part or how
> strcmp(3) works :-). It's much better to be verbose, binary operators
> are cheap these days and the code makes it look like strcmp returns a
> boolean.

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_*().

> 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?

Danny
Cheers,

Danny