Myles Watson wrote:
-----Original Message----- From: Stefan Reinauer [mailto:stepan@coresystems.de] Sent: Wednesday, October 28, 2009 4:25 AM To: Myles Watson Cc: 'Maciej Pijanka'; coreboot@coreboot.org Subject: Re: [coreboot] small patch for prototypes and unused variables
Myles Watson wrote:
Maciej,
Thanks for the patch. I think most of it is ready to be committed.
Index: src/lib/clog2.c
--- src/lib/clog2.c (revision 4869) +++ src/lib/clog2.c (working copy) @@ -7,6 +7,8 @@ /* Assume 8 bits per byte */ #define CHAR_BIT 8
+unsigned long log2(unsigned long x);
unsigned long log2(unsigned long x) { // assume 8 bits per byte.
Things like this make me wonder if we should just turn off the warning.
Is
there a header file where we can put some of these prototypes?
No, we should not turn off the warning. It's the "Is our API correct?" warning. We had very ugly bugs because of different prototypes in different .c files.
In that case we ought to squash them all. Too many warnings makes it hard to see important ones.
Agreed. The old build system made it far to easy to ignore warnings. Now with Kconfig we have a serious chance of detecting issues with our code. :-)
I think this is very healthy.
Yes, there are many header files,.. check src/include for a fitting one. If there is none, we should think about creating one.
Maybe we should create src/include/lib.h for all of the missing ones from src/lib?
Stuff like sprintf would nicely fit in stdio or string.h, so the include file hints in the man pages for those functions kind of fit. Maybe that's odd taste, I don't know.
All the other stuff that does not implement libc functionality (or explicitly deserves an own include file for some reason) should go into a lib.h.
Maybe even just having lib.h is enough.