[coreboot] small patch for prototypes and unused variables
Stefan Reinauer
stepan at coresystems.de
Wed Oct 28 16:02:11 CET 2009
Myles Watson wrote:
>
>> -----Original Message-----
>> From: Stefan Reinauer [mailto:stepan at coresystems.de]
>> Sent: Wednesday, October 28, 2009 4:25 AM
>> To: Myles Watson
>> Cc: 'Maciej Pijanka'; coreboot at 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.
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
More information about the coreboot
mailing list