enabling W=1 by default

[Ilias Apalodimas]

Hi Andy

On Wed, 23 Oct 2024 at 17:52, Andy Shevchenko
<andriy.shevchenko at linux.intel.com> wrote:
>
> On Wed, Oct 23, 2024 at 09:52:09AM +0200, Alexander Dahl wrote:
> > Am Tue, Oct 22, 2024 at 04:23:07PM +0300 schrieb Andy Shevchenko:
> > > On Mon, Oct 21, 2024 at 06:32:21PM +0200, Simon Glass wrote:
> > > > On Mon, 21 Oct 2024 at 16:27, Andy Shevchenko
> > > > <andriy.shevchenko at linux.intel.com> wrote:
> > > > >
> > > > > looking at the redness of the output of `make W=1` here is the question:
> > > > > isn't it a good time to enable `make W=1` by default. Yes, I understand
> > > > > the impact, but at least we can do it mandatory for a _new_ code submitted to
> > > > > U-Boot, right?

I think this is a good idea and perhaps we can do it per subsystem. I
cleaned up EFI a few years ago, but some new have crept in.
I've sent a bunch of no brainer patches, once I clean all of it (and
assuming it's doable), I 'll try enabling W=1 as the default for
efi_loader

thanks
/Ilias
> > > > >
> > > > > Ideally I would have what Linux kernel has for a few releases already, i.e.
> > > > > Werror by default and getting close to make a clean builds with that and
> > > > > make W=1` at least against default configurations (yeah, with U-Boot there is
> > > > > probably no default, but sandbox one).
> > > >
> > > > Warnings should be warnings...
> > >
> > > Yes, and ideally the code should not have warnings, right?
> >
> > +1
> >
> > > Otherwise how can we do better? It's quite similar to what you wrote WRT
> > > documenting the function prototypes, the same applies to the new contribution
> > > WRT `make W=1`.
> > >
> > > > if you would like to enable it for CI that is fine by me,
> > >
> > > Yes, that's the idea, but I'm not the owner of any U-Boot CIs,
> > > hence it's a proposal.
> > >
> > > > but the U-Boot makefile shouldn't do it. It defeats the purpose of
> > > > having a distinction between errors and warnings.
> > >
> > > While it's not what I wanted, I disagree on your comment. The idea is to make
> > > rules stricter (for new code) to make it better and that's why Linus enabled
> > > Werror by default in the Linux kernel. And personally I consider that as a good
> > > thing to follow.
> >
> > Long term experience: each time you upgrade your toolchain you get new
> > warnings.  Each package (u-boot, kernel, userland, does not matter
> > which) enabling -Werror breaks the BSP build.  What should a developer
> > do then?  Fix each warning in each foreign project and bring it
> > upstream?  Or disable -Werror?  Last thing is what is usually done.
> > You can see several patches in buildroot or ptxdist disabling -Werror
> > for this reason.
>
> That's why the set of enabled/disabled warnings are spread over W=<n> and hence
> hidden when known to be PITA. W=1 is kinda special in a sense that we put the
> warnings that might affect code generation, size of the binary, etc. In some cases
> it even might prevent security bugs.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>