[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.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866





More information about the coreboot mailing list