acpi updates

[Richard Hughes]

On Tue, 01 Feb 2005 00:41:10 -0500, David Zeuthen wrote:
> So I had a look at this today and tonight. Some general comments
> 
>  o Missing CVSID: $Id$ in files.

Never used this before... Do I just update the time and date
manually in each patch? Or leave it all to cvs?
 
>  o Missing #ifndef guards for header files

Oops.
 
>  o Header files shouldn't include config.h. Source files should
>    indeed include config.h (with HAVE_CONFIG_H guards)

Oops again.
 
>  o Please use named parameters in header file prototypes, e.g.
> 
>     void foo (const char *someParam, int someOtherParam)
> 
>    rather than
> 
>     void foo (const char *, int)

CodingStyle I guess... I'll play it your way.
 
>  o Coding standards; basically
> 
>     - if ( foo ) should be if (foo); and
>     - some_function() should be some_function () - Use tabs of width 8 -
>     120 characters per line maximum; don't cram things
>       into 80 characters
>     - UNIX text format; no \r\n, only \n

Your points written down and understood.
Not sure why \r\n - maybe when I uploaded it to my webserver.

>  o Please use the Doxygen format for documenting functions instead
>    of a non-standard format (and you don't need to document trivial
>    internal functions)

Sure, again looks like I'll have to learn Doxygen... :-)
 
>  o License boilerplate; please include the full GPLv2 license like
>    the other files in the project

Point.
 
>  o You exported a lot of functions you didn't need to (e.g.
>    procfs_acpi.h) - this confused me a bit.

Ahh, all the world for the want of the word "static" :-) My Bad.

> I fixed all. 

You're a legend. Now I'll have to read my code, trying to understand how
to do things properly!

> I also ripped out the caching parts since it made things
> more unreadable for no real gain. Instead, I've added some new functions
> to util.c that does all the heavy-lifting. 

Cool, I thought this might need your help.

> I also ripped out the PMU and
> APM files since it was only boiler- plate. And I made a few other
> changes and I only ported battery, button and ac_adapter over.

I'll do processor and stuff if you want me to. Plus I've some PMU bits
that need testing.

> So, uhm, the patch I committed is very different from the patch you
> supplied. It's also a bit shorter and IMHO a bit more readable :-) -
> please have a look and let me know what you think.

If if it's in HAL, then I'm happy... I'm just programmer that's learning
all the time.
 
> For fun, I added some polling, it works (you need to run
> ./hal-device-manager from the tools directory; you may need to chmod a+x
> that file), but we need to replace it by listening to the ACPI socket in
> a not too distant future.

Sure I can send you my experimental acpid socket / event code tonight and
see what people think.
 
Thanks again David.

Richard.


(p.s. more patches to follow... :-)

-- 

http://www.hughsie.com/PUBLIC-KEY


_______________________________________________
hal mailing list
hal at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal