On 5/11/20 3:59 AM, Masahiro Yamada wrote:
Hi Simon,
On Mon, May 11, 2020 at 5:37 AM Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On Sat, 9 May 2020 at 05:00, Masahiro Yamada masahiroy@kernel.org wrote:
On Sat, May 9, 2020 at 3:16 AM Tom Rini trini@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@kernel.org wrote:
On Fri, May 8, 2020 at 10:39 AM Simon Glass sjg@chromium.org wrote: > > Hi Masahiro, > > On Thu, 7 May 2020 at 06:21, Masahiro Yamada > yamada.masahiro@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@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.
I really don't understand the point of warnings if we are just going to turn them into errors.
How about adding an option to tell U-Boot to use -Werror, etc.? Then those that what it can enable it.
OK. We can do it with
make KCFLAGS=-Werror
I've had a few of these failures due to implicit fn declaration, so I'd be all for adding the error by default. And if things error out and you are too lazy to spot the error, use make -k ; make -k and the error will be right there at the end.
So I'm fine with this patch as-is.