On Thu, May 25, 2023 at 08:37:07AM -0700, Kees Cook wrote: > Hi! > > On Wed, May 24, 2023 at 04:27:31PM -0400, Hamza Mahfooz wrote: > > + Kees > > > > On 5/24/23 15:50, Alex Deucher wrote: > > > On Wed, May 24, 2023 at 3:46 PM Felix Kuehling <felix.kuehling@xxxxxxx> wrote: > > > > > > > > Sure, I think we tried enabling warnings as errors before and had to > > > > revert it because of weird compiler quirks or the variety of compiler > > > > versions that need to be supported. > > > > > > > > Alex, are you planning to upstream this, or is this only to enforce more > > > > internal discipline about not ignoring warnings? > > > > > > I'd like to upstream it. Upstream already has CONFIG_WERROR as a > > > config option, but it's been problematic to enable in CI because of > > > various breakages outside of the driver and in different compilers. > > > That said, I don't know how much trouble enabling it will cause with > > > various compilers in the wild. > > -Wmisleading-indentation is already part of -Wall, so this is globally > enabled already. > > -Wunused is enabled under W=1, and it's pretty noisy still. If you can > get builds clean in drm, that'll be a good step towards getting it > enabled globally. (A middle ground with less to clean up might be > -Wunused-but-set-variable) > > I agree about -Werror: just stick with CONFIG_WERROR instead. There is also W=e, added by commit c77d06e70d59 ("kbuild: support W=e to make build abort in case of warning") in 5.19, which works well for building with configurations that do not have CONFIG_WERROR enabled and avoiding dipping into menuconfig. Unconditionally enabling -Werror with no way to turn it off is just asking for problems over time with new compiler versions, either due to new warnings in -Wall or warnings that have been improved or changed. Should that still be desired, consider doing what i915 and PowerPC have done and add a Kconfig option that can be disabled. Cheers, Nathan