Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190515043753.9853-1-yamada.masahiro@socionext.com> <155790139253.21839.18331243986827594086@skylake-alporthouse-com>
In-Reply-To: <155790139253.21839.18331243986827594086@skylake-alporthouse-com>
From:   Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Wed, 15 May 2019 19:45:11 +0900
Message-ID: <CAK7LNARu1vGhfzSU9k878H7=1cbD+mD99YOHOL4jBvrmoBTBmg@mail.gmail.com>
Subject: Re: [PATCH] drm/i915: drop unneeded -Wall addition
To:     Chris Wilson <chris@chris-wilson.co.uk>
Cc:     Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        intel-gfx@lists.freedesktop.org, Dave Airlie <airlied@redhat.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, May 15, 2019 at 3:25 PM Chris Wilson <chris@chris-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@socionext.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