[PATCH] kbuild: add -Werror=implicit-function-declaration

[Masahiro Yamada]

On Sat, May 9, 2020 at 3:16 AM Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, May 07, 2020 at 09:16:40PM -0600, Simon Glass wrote:
> > Hi Masahiro,
> >
> > On Thu, 7 May 2020 at 19:54, Masahiro Yamada <masahiroy at kernel.org> wrote:
> > >
> > > On Fri, May 8, 2020 at 10:39 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Masahiro,
> > > >
> > > > On Thu, 7 May 2020 at 06:21, Masahiro Yamada
> > > > <yamada.masahiro at socionext.com> wrote:
> > > > >
> > > > > Add -Werror=implicit-function-declaration as Linux does.
> > > > >
> > > > > If you do not check the prototype, it may go wrong run-time.
> > > > > It is better to break the build, and require to include correct
> > > > > headers.
> > > > >
> > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> > > > > ---
> > > > >
> > > > >  Makefile | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > NAK
> > > >
> > > > We already get a warning in this situation. This makes it painful for
> > > > development since things that should be warnings end up being errors.
> > > > So your build fails when really it should work well enough to do a
> > > > test run with your new code. I don't think it has any benefit on code
> > > > quality since we already detect warnings in gitlab, etc.
> > > >
> > > > U-Boot is set up so that warnings are reported and are easy to spot if
> > > > you use 'make -s' (i.e. not buried in the output).
> > > >
> > > > Regards,
> > > > Simon
> > >
> > >
> > >
> > > Linux added this flag in 2007.
> > >
> > > The intention seems to break the build earlier
> > > when a non-existing function is used.
> > >
> > > I have not seen compliant about this flag in Linux.
> > > What does it make different for U-Boot ?
> >
> > Well that commit message is quite misleading. The author is presumably
> > ignoring the warnings that come out in the compile phase. For me they
> > come up loud and clear. I don't know why it takes half an hour to get
> > to the link stage. My average incremental build time is under 4
> > seconds including the link.
> >
> > Finally, the warning does not tell you anything about whether the
> > function doesn't exist. It just tells you you have left out a header
> > file.
> >
> > I know how much of a pain this is, because coreboot does this. It does
> > it partly because there is so much build output that the warnings are
> > invisible unless they actually halt the build. Even then you have to
> > search for what went wrong.
>
> I'm not immediately sure of the right answer here.  Part of the problem
> is that even with 'make -s' U-Boot can be horribly noisy due to device
> tree warnings.  I assume Yamada-san ran in to a problem and was
> expecting the build to have failed but instead was chasing down a
> run-time debug until finding this.


I did not run into a problem.

When I was replacing <common.h> with some lighter headers,
I missed some warnings ( but I noticed them after all).

In Linux, if I miss to include a header, it fails to build.
In U-Boot, it does not.

Personally, I like to align with Linux policy,
but if you are not comfortable with this patch,
please feel free to ignore it.



>  It's really easy to build with
> -Werror set via buildman, but a lot of people don't expect to have to
> use buildman (as it's not required, just a good idea), see for example
> the thread about building non-functional sunxi binaries.  If they used
> buildman the non-zero exit code would have saved them the debug time.
>
> All that said, I can imagine that doing something like the include
> cleanup series that you do would be even harder with this on.  But on
> the 3rd or 4th hand, adding -k to make gets those builds going along
> anyhow.
>
> Personally, when I see those warnings I fix them up before tossing at
> the hardware, but I know that's not everyones workflow.
>
> --
> Tom



-- 
Best Regards
Masahiro Yamada