On Mon, May 11, 2020 at 02:28:48PM -0600, Simon Glass wrote: > Hi, > > On Sun, 10 May 2020 at 20:14, Marek Vasut <ma...@denx.de> wrote: > > > > On 5/11/20 3:59 AM, Masahiro Yamada wrote: > > > Hi Simon, > > > > > > On Mon, May 11, 2020 at 5:37 AM Simon Glass <s...@chromium.org> wrote: > > >> > > >> Hi Masahiro, > > >> > > >> On Sat, 9 May 2020 at 05:00, Masahiro Yamada <masahi...@kernel.org> > > >> wrote: > > >>> > > >>> On Sat, May 9, 2020 at 3:16 AM Tom Rini <tr...@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 <masahi...@kernel.org> > > >>>>> wrote: > > >>>>>> > > >>>>>> On Fri, May 8, 2020 at 10:39 AM Simon Glass <s...@chromium.org> > > >>>>>> wrote: > > >>>>>>> > > >>>>>>> Hi Masahiro, > > >>>>>>> > > >>>>>>> On Thu, 7 May 2020 at 06:21, Masahiro Yamada > > >>>>>>> <yamada.masah...@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.masah...@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 are you ignoring the warning? Is it because there are so many > device-tree warnings? Should we figure out how to silence those > things?
I keep wondering how many device tree warnings would get fixed with a re-sync of Linux. -- Tom
signature.asc
Description: PGP signature