Re: [coreboot] [RFC] Error out on implicit declarations

[Myles Watson]

On Thu, Dec 18, 2008 at 11:08 AM, Marc Jones  wrote:
> On Thu, Dec 18, 2008 at 10:22 AM, Myles Watson  wrote:
> > On Thu, Dec 18, 2008 at 7:52 AM, Jordan Crouse <[e-mail]
> >
> > wrote:
> > > Myles Watson wrote:
> > > > On Thu, Dec 18, 2008 at 3:18 AM, Carl-Daniel Hailfinger <
> > > > [e-mail]> 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
>
> Acked-by: Marc Jones

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