On Thu, Aug 29, 2024 at 08:00:19AM -0700, Junio C Hamano wrote: > > Yes, it would be fine to use MAYBE_UNUSED in a case like this. > > It turns out that I was, without realizing it myself, making an > oblique reference to your patch 7/6 ;-) > > Perhaps something along this line? Yeah, this looks good. A few small comments below (but I'm not sure anything needs to be changed). > diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines > index d0fc7cfe60..3263245b03 100644 > --- c/Documentation/CodingGuidelines > +++ w/Documentation/CodingGuidelines > @@ -262,8 +262,9 @@ For C programs: > like "error: unused parameter 'foo' [-Werror=unused-parameter]", > which indicates that a function ignores its argument. If the unused > parameter can't be removed (e.g., because the function is used as a > - callback and has to match a certain interface), you can annotate the > - individual parameters with the UNUSED keyword, like "int foo UNUSED". > + callback and has to match a certain interface), you can annotate > + the individual parameters with the UNUSED (or MAYBE_UNUSED) > + keyword, like "int foo UNUSED". Here I was going to suggest explaining why you'd use one or the other (because I'm afraid of people using MAYBE_UNUSED when UNUSED would be more appropriate). But I think the extra comments you added later are even better, as it lets us explain without cluttering up the CodingGuidelines document. > +/* > + * UNUSED marks a function parameter that is always unused. > + * > + * A callback interface may dictate that a function accepts a > + * parameter at that position, but the implementation of the function > + * may not need to use the parameter. In such a case, mark the parameter > + * with UNUSED. > + * > + * When a parameter may be used or unused, depending on conditional > + * compilation, consider using MAYBE_UNUSED instead. > + */ Looks good. > +/* > + * MAYBE_UNUSED marks a function parameter that may be unused, but > + * whose use is not an error. > + * > + * Depending on a configuration, all uses of a function parameter may > + * become #ifdef'ed away. Marking such a parameter with UNUSED would > + * give a warning in a compilation where the parameter is indeed used, > + * and not marking such a parameter would give a warning in a > + * compilation where the parameter is unused. > + */ > #define MAYBE_UNUSED __attribute__((__unused__)) This is all good as pertains to function parameters. But the original reason we added MAYBE_UNUSED was actually for static functions that were auto-generated by the commit-slab macros. Saying "...marks a function parameter" implies to me that it's the only use. I don't know if we want to be more expansive here or not. Adding auto-generated macro functions should be quite a rarity, I'd think. -Peff
