[coreboot] [RFC] Error out on implicit declarations

[Myles Watson]

On Thu, Dec 18, 2008 at 11:08 AM, Marc Jones <marcj303 at gmail.com> wrote:

> On Thu, Dec 18, 2008 at 10:22 AM, Myles Watson <mylesgw at gmail.com> wrote:
> >
> >
> > On Thu, Dec 18, 2008 at 7:52 AM, Jordan Crouse <jordan at cosmicpenguin.net
> >
> > wrote:
> >>
> >> Myles Watson wrote:
> >>>
> >>> On Thu, Dec 18, 2008 at 3:18 AM, Carl-Daniel Hailfinger <
> >>> c-d.hailfinger.devel.2006 at gmx.net> wrote:
> >>>
> >>>> Bao, Zheng found a bug which killed SATA booting on my board.
> >>>>
> >>>> This happened because we do not error out on implicit function
> >>>> declarations. The linker has no way of checking whether the implicitly
> >>>> assumed function signature is identical to the real signature, so
> >>>> mismatches can occur and these mismatches are practically impossible
> to
> >>>> debug because the code looks completely correct.
> >>>>
> >>>> Adding -Werror-implicit-function-declaration to our CFLAGS would solve
> >>>> this problem nicely, but a lot of files in the tree need to be fixed.
> >>>>
> >>>
> >>> I think this is a great idea.  Isn't the correct order to fix all the
> >>> warnings, then make it an error?
> >>
> >> Yeah - the unfortunate thing about changes like this is that you end up
> >> being responsible for fixing the errors.. :)
> >
> > Here's my first patch.  It clears up all of them except get_nodes for
> > serengeti.
> >
> > coreboot/svn/src/cpu/amd/dualcore/dualcore.c:63: warning: implicit
> > declaration of function 'get_nodes'
> >
> > The rest were easy.  This one I'm not sure what was supposed to be here.
> >
> >>
> >> Carl-Daniel - if you post a list of offending files, we'll all help
> clear
> >> them up.  Dumping the log through grep "implicit declaration of
> function"
> >> should suffice.
> >
> > If you want to take the get_nodes reference, that would be great.  If
> this
> > is the way its supposed to be cleaned up, I'll keep going a little more.
>  I
> > think we should divide it up based on processor type so we don't
> duplicate
> > work.
> >
> > Signed-off-by: Myles Watson <mylesgw at gmail.com>
> Acked-by: Marc Jones <marcj303 at gmail.com>


Rev 3818.  Thanks.


> This is a much needed cleanup. Thanks Carl-Daniel and Myles for starting on
> it.
> I think that the get_node is a little tricky because  it is used in
> CAR and in main coreboot code?
>

It's also inlined some places but not others.

I'm attaching a patch which I thought was trivial, but breaks things all
over the place.  I'm not sure how this will need to be done.  It looks like
a problem with romcc vs. CAR.

Thanks,
Myles
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081218/8cfbbba5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pci_implicit.diff
Type: text/x-patch
Size: 3352 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081218/8cfbbba5/attachment.diff>