Re: [PATCH] drm/i915: drop unneeded -Wall addition

From: Masahiro Yamada
Date: Wed May 15 2019 - 06:47:57 EST

On Wed, May 15, 2019 at 3:25 PM Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> Quoting Masahiro Yamada (2019-05-15 05:37:53)
> > The top level Makefile adds -Wall globally:
> >
> > KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
> >
> > I see two "-Wall" added for compiling under drivers/gpu/drm/i915/.
> Does it matter? Is the statement in i915/Makefile not more complete for
> saying "-Wall -Wextra -Werror"?

Not fatal, but better to fix.

Why not fix the comment if you mind
"-Wall" in the comment?

It will be easy to rephrase the comments
without explicitly mentioning -Wall or -Wextra.

I reworded it more concisely:

# We aggressively eliminate warnings,
# so here are more warning options than default.

That's it.

The CI is your local matter.
Distracting comments should not be added in the upstream code
in the first place.

> > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > ---
> >
> > BTW, I have a question in the comment:
> >
> > "Note the danger in using -Wall -Wextra is that when CI updates gcc we
> > will most likely get a sudden build breakage... Hopefully we will fix
> > new warnings before CI updates!"
> >
> > Enabling whatever warning options does not cause build breakage.
> > -Werror does.
> >
> > So, I think the correct statement is:
> >
> > "Note the danger in using -Werror is that when CI updates gcc we ...
> No.

Heh, I thought the answer was Yes,
since I saw the following in this Makefile.

# Add a set of useful warning flags and enable -Werror for CI to prevent

> CI enforces -Werror and that is constant, so the uncontrolled
> variable, the danger, lies in using the unreliable heuristics gcc may
> arbitrary enable between versions. That the set of warnings causing an
> error may be different between CI and the developer.
> -Chris

Best Regards
Masahiro Yamada