Mega ACPI and PMU cleanup

Danny Kukawka danny.kukawka at web.de
Sun Feb 26 14:12:39 PST 2006


On Sunday 26 February 2006 22:18, David Zeuthen wrote:
> I haven't looked much into the details of the patch since yourself and
> others are probably more familiar with this code. Go ahead and commit it
> we'll figure out soon enough if something is wrong.

Sorry (don't missunderstand this), but is this a joke? Commit it and we see 
soon enough if something is wrong? Maybe after the next release? ;-)

Btw. I taked today a look at the patch. I already maked comments in fd.o bug 
#5752. 

This patch change hal_util_grep_int_elem_from_file in utils.c (to return 
success of the function instead of the requested value) and add 
hal_util_grep_bool_elem_from_file (with the same synthax) but the same kind 
of function - hal_util_grep_string_elem_from_file - isn't change. Different 
handle/parameter for the same kind of function is IMO not a good idea.

In the code are many unneeded checks like this:

res = hal_util_grep_int_elem_from_file (...);
if (res)
	hal_device_property_set_int (...);

This blow up the code, add a not needed variable, is IMO not the best code 
style and you loose the advantages of the changes in utils.c. Better do this:

if (hal_util_grep_int_elem_from_file (...))
	hal_device_property_set_int (...);


Btw. I didn't checked the code for logical errors or made any other QA checks 
with different ACPI machines.

Cheers,

Danny


More information about the hal mailing list