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

[Simon Glass]

Hi Tom,

On Fri, 8 May 2020 at 12:16, 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

Yes, I even added -y to buildman as I was so sick of that.

> expecting the build to have failed but instead was chasing down a
> run-time debug until finding this.  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

Yes, in general. Errors make the pace of progress really slow.
Warnings are not as bad. You know you need to get them them but at
least you can see the whole picture of what you are doing.

> the 3rd or 4th hand, adding -k to make gets those builds going along
> anyhow.

Yes there are lots of workarounds, but warnings are warnings for a
reason. The code can still work, and you need to fix it sometime.

>
> Personally, when I see those warnings I fix them up before tossing at
> the hardware, but I know that's not everyones workflow.

Generally for me, building = writing to the hardware, so long as the
build doesn't fail. Normally my build step is 'make' && 'write to
board' so I just need to press reset if that is not in the script.

Regards,
Simon