Add -Wunused family of flags and fix all the warnings coming because of
argc and argv. Remove them if they aren't being used entirely. Use
__unused compiler attribute with argc where argv is being used.
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
tools/testing/selftests/mm/Makefile | 2 +-
tools/testing/selftests/mm/compaction_test.c | 2 +-
tools/testing/selftests/mm/cow.c | 2 +-
tools/testing/selftests/mm/droppable.c | 2 +-
tools/testing/selftests/mm/gup_longterm.c | 2 +-
tools/testing/selftests/mm/hugepage-vmemmap.c | 2 +-
tools/testing/selftests/mm/hugetlb-madvise.c | 2 +-
tools/testing/selftests/mm/hugetlb-soft-offline.c | 2 +-
tools/testing/selftests/mm/madv_populate.c | 2 +-
tools/testing/selftests/mm/map_populate.c | 2 +-
tools/testing/selftests/mm/memfd_secret.c | 2 +-
tools/testing/selftests/mm/mlock-random-test.c | 2 +-
tools/testing/selftests/mm/mlock2-tests.c | 2 +-
tools/testing/selftests/mm/on-fault-limit.c | 2 +-
tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 +-
tools/testing/selftests/mm/soft-dirty.c | 2 +-
tools/testing/selftests/mm/uffd-wp-mremap.c | 2 +-
tools/testing/selftests/mm/virtual_address_range.c | 2 +-
18 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 23d4bf6215465..d75f1effcb791 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -20,7 +20,6 @@ endif
# thus tricking Make (and you!) into believing that All Is Well, in subsequent
# make invocations:
.DELETE_ON_ERROR:
-
# Avoid accidental wrong builds, due to built-in rules working just a little
# bit too well--but not quite as well as required for our situation here.
#
@@ -35,6 +34,7 @@ MAKEFLAGS += --no-builtin-rules
CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
CFLAGS += -Wunreachable-code
+CFLAGS += -Wunused -Wunused-parameter -Wunused-function -Wunused-label -Wunused-variable -Wunused-value
LDLIBS = -lrt -lpthread -lm
# Some distributions (such as Ubuntu) configure GCC so that _FORTIFY_SOURCE is
diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 9bc4591c7b169..4fa03679e9b07 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -203,7 +203,7 @@ int set_zero_hugepages(unsigned long *initial_nr_hugepages)
return ret;
}
-int main(int argc, char **argv)
+int main(void)
{
struct rlimit lim;
struct map_list *list = NULL, *entry;
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index d30625c18259b..eb1ccf067b633 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -1857,7 +1857,7 @@ static int tests_per_non_anon_test_case(void)
return tests;
}
-int main(int argc, char **argv)
+int main(void)
{
struct thp_settings default_settings;
diff --git a/tools/testing/selftests/mm/droppable.c b/tools/testing/selftests/mm/droppable.c
index f3d9ecf96890a..90ea6377810c5 100644
--- a/tools/testing/selftests/mm/droppable.c
+++ b/tools/testing/selftests/mm/droppable.c
@@ -15,7 +15,7 @@
#include "../kselftest.h"
-int main(int argc, char *argv[])
+int main(void)
{
size_t alloc_size = 134217728;
size_t page_size = getpagesize();
diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
index 268dadb8ce438..7fe4f94400cb6 100644
--- a/tools/testing/selftests/mm/gup_longterm.c
+++ b/tools/testing/selftests/mm/gup_longterm.c
@@ -504,7 +504,7 @@ static int tests_per_test_case(void)
return 3 + nr_hugetlbsizes;
}
-int main(int argc, char **argv)
+int main(void)
{
int i;
diff --git a/tools/testing/selftests/mm/hugepage-vmemmap.c b/tools/testing/selftests/mm/hugepage-vmemmap.c
index df366a4d1b92d..23e97e552057d 100644
--- a/tools/testing/selftests/mm/hugepage-vmemmap.c
+++ b/tools/testing/selftests/mm/hugepage-vmemmap.c
@@ -87,7 +87,7 @@ static int check_page_flags(unsigned long pfn)
return 0;
}
-int main(int argc, char **argv)
+int main(void)
{
void *addr;
unsigned long pfn;
diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
index 1afe14b9dc0c3..27c8e46ae9b7d 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -55,7 +55,7 @@ void read_fault_pages(void *addr, unsigned long nr_pages)
}
}
-int main(int argc, char **argv)
+int main(int __unused argc, char **argv)
{
unsigned long free_hugepages;
void *addr, *addr2;
diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c b/tools/testing/selftests/mm/hugetlb-soft-offline.c
index f086f0e04756f..cb087303f5ed3 100644
--- a/tools/testing/selftests/mm/hugetlb-soft-offline.c
+++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c
@@ -216,7 +216,7 @@ static void test_soft_offline_common(int enable_soft_offline)
enable_soft_offline);
}
-int main(int argc, char **argv)
+int main(void)
{
ksft_print_header();
ksft_set_plan(2);
diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
index b6fabd5c27ed6..178e0ae0cd4a1 100644
--- a/tools/testing/selftests/mm/madv_populate.c
+++ b/tools/testing/selftests/mm/madv_populate.c
@@ -281,7 +281,7 @@ static int system_has_softdirty(void)
#endif
}
-int main(int argc, char **argv)
+int main(void)
{
int nr_tests = 16;
int err;
diff --git a/tools/testing/selftests/mm/map_populate.c b/tools/testing/selftests/mm/map_populate.c
index 9df2636c829bf..2b240499f15c9 100644
--- a/tools/testing/selftests/mm/map_populate.c
+++ b/tools/testing/selftests/mm/map_populate.c
@@ -76,7 +76,7 @@ static int child_f(int sock, unsigned long *smap, int fd)
return ksft_cnt.ksft_pass;
}
-int main(int argc, char **argv)
+int main(void)
{
int sock[2], child, ret;
FILE *ftmp;
diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
index 9a0597310a765..836383f63b630 100644
--- a/tools/testing/selftests/mm/memfd_secret.c
+++ b/tools/testing/selftests/mm/memfd_secret.c
@@ -299,7 +299,7 @@ static void prepare(void)
#define NUM_TESTS 6
-int main(int argc, char *argv[])
+int main(void)
{
int fd;
diff --git a/tools/testing/selftests/mm/mlock-random-test.c b/tools/testing/selftests/mm/mlock-random-test.c
index b8d7e966f44c6..4ff7a4cfc7331 100644
--- a/tools/testing/selftests/mm/mlock-random-test.c
+++ b/tools/testing/selftests/mm/mlock-random-test.c
@@ -236,7 +236,7 @@ static void test_mlock_outof_limit(char *p, int alloc_size)
ksft_test_result_pass("%s\n", __func__);
}
-int main(int argc, char **argv)
+int main(void)
{
char *p = NULL;
diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c
index 3e90ff37e336a..ce5fd5ce1f51f 100644
--- a/tools/testing/selftests/mm/mlock2-tests.c
+++ b/tools/testing/selftests/mm/mlock2-tests.c
@@ -425,7 +425,7 @@ static void test_mlockall(void)
munlockall();
}
-int main(int argc, char **argv)
+int main(void)
{
int ret, size = 3 * getpagesize();
void *map;
diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
index 431c1277d83a1..ade160966c926 100644
--- a/tools/testing/selftests/mm/on-fault-limit.c
+++ b/tools/testing/selftests/mm/on-fault-limit.c
@@ -28,7 +28,7 @@ static void test_limit(void)
munlockall();
}
-int main(int argc, char **argv)
+int main(void)
{
ksft_print_header();
ksft_set_plan(1);
diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index 302fef54049c8..eb4ef8532c0bf 100644
--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
+++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -528,7 +528,7 @@ static void (*pkey_tests[])(void) = {
test_pkru_sigreturn
};
-int main(int argc, char *argv[])
+int main(void)
{
int i;
diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/selftests/mm/soft-dirty.c
index 8a3f2b4b21869..e62be4136f69e 100644
--- a/tools/testing/selftests/mm/soft-dirty.c
+++ b/tools/testing/selftests/mm/soft-dirty.c
@@ -194,7 +194,7 @@ static void test_mprotect_file(int pagemap_fd, int pagesize)
test_mprotect(pagemap_fd, pagesize, false);
}
-int main(int argc, char **argv)
+int main(void)
{
int pagemap_fd;
int pagesize;
diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c
index c2ba7d46c7b45..13ceb56289701 100644
--- a/tools/testing/selftests/mm/uffd-wp-mremap.c
+++ b/tools/testing/selftests/mm/uffd-wp-mremap.c
@@ -334,7 +334,7 @@ static const struct testcase testcases[] = {
},
};
-int main(int argc, char **argv)
+int main(void)
{
struct thp_settings settings;
int i, j, plan = 0;
diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
index 169dbd692bf5f..3c21d136962cb 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -189,7 +189,7 @@ static int validate_complete_va_space(void)
return 0;
}
-int main(int argc, char *argv[])
+int main(void)
{
char *ptr[NR_CHUNKS_LOW];
char **hptr;
--
2.39.5On 31/07/2025 18:01, Muhammad Usama Anjum wrote: > [...] > > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile > index 23d4bf6215465..d75f1effcb791 100644 > --- a/tools/testing/selftests/mm/Makefile > +++ b/tools/testing/selftests/mm/Makefile > @@ -20,7 +20,6 @@ endif > # thus tricking Make (and you!) into believing that All Is Well, in subsequent > # make invocations: > .DELETE_ON_ERROR: > - > # Avoid accidental wrong builds, due to built-in rules working just a little > # bit too well--but not quite as well as required for our situation here. > # > @@ -35,6 +34,7 @@ MAKEFLAGS += --no-builtin-rules > > CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES) > CFLAGS += -Wunreachable-code > +CFLAGS += -Wunused -Wunused-parameter -Wunused-function -Wunused-label -Wunused-variable -Wunused-value -Wall implies all of these except -Wunused-parameter (at least according to gcc(1)). 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. - Kevin
On 8/18/25 1:16 PM, Kevin Brodsky wrote: > On 31/07/2025 18:01, Muhammad Usama Anjum wrote: >> [...] >> >> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile >> index 23d4bf6215465..d75f1effcb791 100644 >> --- a/tools/testing/selftests/mm/Makefile >> +++ b/tools/testing/selftests/mm/Makefile >> @@ -20,7 +20,6 @@ endif >> # thus tricking Make (and you!) into believing that All Is Well, in subsequent >> # make invocations: >> .DELETE_ON_ERROR: >> - >> # Avoid accidental wrong builds, due to built-in rules working just a little >> # bit too well--but not quite as well as required for our situation here. >> # >> @@ -35,6 +34,7 @@ MAKEFLAGS += --no-builtin-rules >> >> CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES) >> CFLAGS += -Wunreachable-code >> +CFLAGS += -Wunused -Wunused-parameter -Wunused-function -Wunused-label -Wunused-variable -Wunused-value > > -Wall implies all of these except -Wunused-parameter (at least according > to gcc(1)). I'll remove others in separate patch. > > 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. > > - Kevin -- --- Thanks, Usama
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. - Kevin
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
On Thu, 21 Aug 2025 17:31:21 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > 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? I think a new series would be clearer, please. It's unclear (to me) whether Kevin's various comments have all been addressed.
On Mon, 18 Aug 2025 10:16:35 +0200 Kevin Brodsky <kevin.brodsky@arm.com> wrote:
> > # Avoid accidental wrong builds, due to built-in rules working just a little
> > # bit too well--but not quite as well as required for our situation here.
> > #
> > @@ -35,6 +34,7 @@ MAKEFLAGS += --no-builtin-rules
> >
> > CFLAGS = -Wall -O2 -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
> > CFLAGS += -Wunreachable-code
> > +CFLAGS += -Wunused -Wunused-parameter -Wunused-function -Wunused-label -Wunused-variable -Wunused-value
>
> -Wall implies all of these except -Wunused-parameter (at least according
> to gcc(1)).
>
> 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.
Yeah, we rely upon unused parameters in a million places:
#else
static inline void some_stub_function(type1 arg2, type2 arg2)
{
}
#endif
© 2016 - 2025 Red Hat, Inc.