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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 15 May 2019 19:45:11 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Chris Wilson <chris@...is-wilson.co.uk>
Cc:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        intel-gfx@...ts.freedesktop.org, Dave Airlie <airlied@...hat.com>,
        Sam Ravnborg <sam@...nborg.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>
Subject: Re: [PATCH] drm/i915: drop unneeded -Wall addition

On Wed, May 15, 2019 at 3:25 PM Chris Wilson <chris@...is-wilson.co.uk> 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@...ionext.com>
> > ---
> >
> > 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