[1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[ende.tan]

From: Tan En De <ende.tan@starfivetech.com>

Mark unused parameters with `__attribute__((unused))`.

Signed-off-by: Tan En De <ende.tan@starfivetech.com>
---
 src/cyclictest/cyclictest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index b1f8420..f2db5e4 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -603,12 +603,12 @@ static int has_smi_counter(void)
 	return 1;
 }
 #else
-static int open_msr_file(int cpu)
+static int open_msr_file(int cpu __attribute__((unused)))
 {
 	return -1;
 }
 
-static int get_smi_counter(int fd, unsigned long *counter)
+static int get_smi_counter(int fd __attribute__((unused)), unsigned long *counter __attribute__((unused)))
 {
 	return 1;
 }
-- 
2.34.1

Re: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[Crystal Wood]

On Fri, 2024-12-13 at 00:24 +0800, ende.tan@starfivetech.com wrote:
> From: Tan En De <ende.tan@starfivetech.com>
> 
> Mark unused parameters with `__attribute__((unused))`.
> 
> Signed-off-by: Tan En De <ende.tan@starfivetech.com>
> ---
>  src/cyclictest/cyclictest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Can we just turn off this warning?  Unused parameters are entirely
normal in functions used as callbacks as well as with ifdefs, and it
seems like a low value warning to begin with (there's a reason it's not
in -Wall).


-Crystal

Re: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[John Kacur]

On Fri, 13 Dec 2024, Crystal Wood wrote:

> On Fri, 2024-12-13 at 00:24 +0800, ende.tan@starfivetech.com wrote:
> > From: Tan En De <ende.tan@starfivetech.com>
> > 
> > Mark unused parameters with `__attribute__((unused))`.
> > 
> > Signed-off-by: Tan En De <ende.tan@starfivetech.com>
> > ---
> >  src/cyclictest/cyclictest.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Can we just turn off this warning?  Unused parameters are entirely
> normal in functions used as callbacks as well as with ifdefs, and it
> seems like a low value warning to begin with (there's a reason it's not
> in -Wall).

I believe it IS in -Wall
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

John

Re: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[John Kacur]

On Fri, 13 Dec 2024, ende.tan@starfivetech.com wrote:

> From: Tan En De <ende.tan@starfivetech.com>
> 
> Mark unused parameters with `__attribute__((unused))`.

You were probably trying to be consistent with the rest of the programs in 
rt-tests, however, according to checkpatch, 

WARNING: __always_unused or __maybe_unused is preferred over 
__attribute__((__unused__))

__always_unused is slightly shorter, shall we try that instead?

John

> 
> Signed-off-by: Tan En De <ende.tan@starfivetech.com>
> ---
>  src/cyclictest/cyclictest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index b1f8420..f2db5e4 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -603,12 +603,12 @@ static int has_smi_counter(void)
>  	return 1;
>  }
>  #else
> -static int open_msr_file(int cpu)
> +static int open_msr_file(int cpu __attribute__((unused)))
>  {
>  	return -1;
>  }
>  
> -static int get_smi_counter(int fd, unsigned long *counter)
> +static int get_smi_counter(int fd __attribute__((unused)), unsigned long *counter __attribute__((unused)))
>  {
>  	return 1;
>  }
> -- 
> 2.34.1
> 
> 
> 

RE: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[EnDe Tan]

> WARNING: __always_unused or __maybe_unused is preferred over
> __attribute__((__unused__))

I tried the `__maybe_unused` using gcc 12, but it didn't work.

It looks like the `__maybe_unused` attribute is only available in C23 and later
https://en.cppreference.com/w/c/language/attributes/maybe_unused

For backward compatibility, maybe we shall keep the existing `__attribute__((__unused__))`.

Regards
Tan En De

Re: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[Crystal Wood]

On Fri, 2024-12-20 at 13:02 -0500, John Kacur wrote:
> 
> On Fri, 13 Dec 2024, Crystal Wood wrote:
> 
> > On Fri, 2024-12-13 at 00:24 +0800, ende.tan@starfivetech.com wrote:
> > > From: Tan En De <ende.tan@starfivetech.com>
> > > 
> > > Mark unused parameters with `__attribute__((unused))`.
> > > 
> > > Signed-off-by: Tan En De <ende.tan@starfivetech.com>
> > > ---
> > >  src/cyclictest/cyclictest.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > Can we just turn off this warning?  Unused parameters are entirely
> > normal in functions used as callbacks as well as with ifdefs, and it
> > seems like a low value warning to begin with (there's a reason it's not
> > in -Wall).
> 
> I believe it IS in -Wall
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

-Wunused-parameter is in -Wextra, as per that link.  It's a crappy
warning that just causes noise to suppress all over the place.  The
kernel doesn't enable this warning, FWIW.

-Crystal

Re: [1/1] rt-tests: cyclictest: Suppress unused parameter warnings

[Crystal Wood]

On Fri, 2024-12-20 at 13:07 -0500, John Kacur wrote:
> 
> On Fri, 13 Dec 2024, ende.tan@starfivetech.com wrote:
> 
> > From: Tan En De <ende.tan@starfivetech.com>
> > 
> > Mark unused parameters with `__attribute__((unused))`.
> 
> You were probably trying to be consistent with the rest of the programs in 
> rt-tests, however, according to checkpatch, 
> 
> WARNING: __always_unused or __maybe_unused is preferred over 
> __attribute__((__unused__))
> 
> __always_unused is slightly shorter, shall we try that instead?

Checkpatch isn't perfect even when used on kernel sources, but should be
taken with quite a few grains of salt when used with other projects. 
Failing to use a macro that doesn't even exist in rt-tests is a bit more
than "trying to be consistent" :-P

-Crystal