On Wed, Aug 28, 2024 at 01:56:13AM -0400, Eric Sunshine wrote: > On Wed, Aug 28, 2024 at 12:01 AM Jeff King <peff@xxxxxxxx> wrote: > > Having now removed or annotated all of the unused function parameters in > > our code base, I found that each instance falls into one of three > > categories: > > > > 1. ignoring the parameter is a bug (e.g., a function takes a ptr/len > > pair, but ignores the length). Detecting these helps us find the > > bugs. > > > > 2. the parameter is unnecessary (and usually left over from a > > refactoring or earlier iteration of a patches series). Removing > > these cleans up the code. > > > > 3. the function has to conform to a specific interface (because it's > > used via a function pointer, or matches something on the other side > > of an #ifdef). These ones are annoying, but annotating them with > > UNUSED is not too bad (especially if the compiler tells you about > > the problem promptly). > > [...] > > And since the code base is now at a spot where we compile cleanly with > > -Wunused-parameter, turning it on will make it the responsibility of > > individual patch writers going forward. > > > > Signed-off-by: Jeff King <peff@xxxxxxxx> > > --- > > diff --git a/config.mak.dev b/config.mak.dev > > @@ -54,7 +54,6 @@ ifeq ($(filter extra-all,$(DEVOPTS)),) > > DEVELOPER_CFLAGS += -Wno-sign-compare > > -DEVELOPER_CFLAGS += -Wno-unused-parameter > > What is the expectation regarding newcomers to the project or even > people who have not been following this topic and its cousins? > Documentation/CodingGuidelines recommends enabling DEVELOPER mode, > which is good, but this change means that such people may now be hit > with a compiler complaint which they don't necessarily know how to > deal with in the legitimate case #3 (described above). Should > CodingGuidelines be updated to mention "UNUSED" and the circumstances > under which it should be used? Updating our coding guidelines would certainly be welcome. Other than that this series looks good to me and is a step into the right direction in my opinion. Thanks! Patrick
