On 8/21/25 3:43 PM, Kevin Brodsky wrote: > On 21/08/2025 08:28, Muhammad Usama Anjum wrote: >>> As to -Wunused-parameter I am frankly not convinced it's worth the >>> hassle. We're getting 90 lines changed in patch 6-8 just to mark >>> parameters as unused, in other words noise to keep the compiler happy. >>> It is not enabled by default in the kernel proper precisely because it >>> is so noisy when callbacks are involved. >>> >>> Patch 5 is clearly an improvement, but I'd rather take it without >>> actually enabling -Wunused-parameter. The rest of this patch isn't that >>> useful either IMHO. >> Patch 5 removes genuinely unused parameters flagged by the compiler. If we >> drop the -Wunused-parameter option, however, new unused parameters will >> continue to creep in with future patches. The goal of enabling this warning >> is to surface such issues early so developers can address them during >> development, rather than later during review or debugging. >> >> Long term, I’d like us to rely more on compiler and static analysis just like >> kernel to catch these kinds of problems proactively, instead of waiting until >> they’re reported or someone fixes them later. While it may feel like noise >> initially, this is largely a one-time cleanup—once done, developers will >> simply fix warnings as they arise, keeping the codebase cleaner going >> forward. > > Agreed on the general principle, but I think the hassle is just too big > for what we're getting in return here (see also Andrew's reply). New > code may also introduce a bunch of unused parameters for legitimate > reasons and it's easy to imagine contributors ignoring such seemingly > harmless/irrelevant warnings instead of sprinkling __unused all over.
> My > feeling is that unused parameters are expected to be allowed in the > kernel and it isn't helpful to go against that expectation in just a > small subset of kselftests. I thought kernel must be giving error for unused parameters as well (from my memory). But just checked and it doesn't seem like it. I'm okay with dropping -Wunused-parameters. We need to drop patch 6/7 and in Patch 4 should have only: -CFLAGS += -Wunused -Wunused-parameter -Wunused-function -Wunused-label -Wunused-variable -Wunused-value +CFLAGS += -Wunused @Andrew, Should I resend the entire series or would you can make the change? > > - Kevin -- --- Thanks, Usama
