On Fri, Oct 06, 2023 at 03:34:47PM +0300, Jani Nikula wrote: > We enable a bunch more compiler warnings than the kernel > defaults. However, they've drifted to become a unique set of warnings, > and have increasingly fallen behind from the W=1 set. > > Align with the W=1 warnings from scripts/Makefile.extrawarn for clarity, > by copy-pasting them with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it > easier to compare in the future. > > Cc: Arnd Bergmann <arnd@xxxxxxxx> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> One meta comment and review comment below. Feel free to carry forward Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx> on future revisions. > --- > > An alternative or future option would be to have Makefile.extrawarn > assign W=1 etc. flags to intermediate variables, say KBUILD_CFLAGS_W1, > like this: > > KBUILD_CFLAGS_W1 += -Wextra -Wunused -Wno-unused-parameter > etc... > > export KBUILD_CFLAGS_W1 > > ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),) > > KBUILD_CFLAGS += $(KBUILD_CFLAGS_W1) > > else > etc... > > and then drivers and subsystems could simply use: > > subdir-ccflags-y += $(KBUILD_CFLAGS_W1) > > to enable and remain up-to-date with W=1 warnings. This has definitely come up a few times and while I am generally in favor of something like this, it makes adding warnings to W=1 a little bit harder because when we add warnings to W=1, we typically are not concerned with breaking the build, as W=1 is not the default build. If a subsystem has opted into "whatever the current W=1 is" by default, changes to W=1 will have to be reviewed/tested within a normal build. Doing something like this patch with a more regular cadence (maybe every update after the merge window) seems like a reasonable compromise to me, although I know that means more work for individual subsystem maintainers. > --- > drivers/gpu/drm/i915/Makefile | 33 ++++++++++++++++++--------------- > 1 file changed, 18 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 623f81217442..0485157054fc 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -3,22 +3,25 @@ > # Makefile for the drm device driver. This driver provides support for the > # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. > > -# Add a set of useful warning flags and enable -Werror for CI to prevent > -# trivial mistakes from creeping in. We have to do this piecemeal as we reject > -# any patch that isn't warning clean, so turning on -Wextra (or W=1) we > -# need to filter out dubious warnings. Still it is our interest > -# to keep running locally with W=1 C=1 until we are completely clean. > -# > -# Note the danger in using -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! > -subdir-ccflags-y := -Wextra > -subdir-ccflags-y += -Wno-unused-parameter > -subdir-ccflags-y += -Wno-type-limits > -subdir-ccflags-y += -Wno-missing-field-initializers > -subdir-ccflags-y += -Wno-sign-compare > -subdir-ccflags-y += -Wno-shift-negative-value As the test robot points out, you'll want to keep these four, as they are only enabled for W=2 or W=3. With this diff on top of these two patches: diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0485157054fc..9c4e09c8aa4e 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -21,6 +21,12 @@ subdir-ccflags-y += $(call cc-option, -Wstringop-overflow) subdir-ccflags-y += $(call cc-option, -Wstringop-truncation) # --- end copy-paste +# The following turn off the warnings enabled by -Wextra +subdir-ccflags-y += -Wno-type-limits +subdir-ccflags-y += -Wno-missing-field-initializers +subdir-ccflags-y += -Wno-sign-compare +subdir-ccflags-y += -Wno-shift-negative-value + # Enable -Werror in CI and development subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror I can continue to build i915 warning free with ARCH=x86_64 allmodconfig using all supported versions of LLVM for building the kernel. > +# Unconditionally enable W=1 warnings locally > +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn > +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter > +subdir-ccflags-y += -Wmissing-declarations > +subdir-ccflags-y += $(call cc-option, -Wrestrict) > +subdir-ccflags-y += -Wmissing-format-attribute > +subdir-ccflags-y += -Wmissing-prototypes > +subdir-ccflags-y += -Wold-style-definition > +subdir-ccflags-y += -Wmissing-include-dirs > subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) > +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable) > +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned) > +subdir-ccflags-y += $(call cc-option, -Wformat-overflow) > +subdir-ccflags-y += $(call cc-option, -Wformat-truncation) > +subdir-ccflags-y += $(call cc-option, -Wstringop-overflow) > +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation) > +# --- end copy-paste > + > +# Enable -Werror in CI and development > subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror > > # Fine grained warnings disable > -- > 2.39.2 >
