[PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[behanw]

From: Mark Charlebois <charlebm@gmail.com>

This code makes a compile time type check that is optimized away. Clang
complains that it generates an unused function.

I believe GCC won't complain for a static inline fuction but would if it
was just a static function.

Adding the unused attribute to the function declaration removes the warning.

This code works for both gcc and clang.

Signed-off-by: Mark Charlebois <charlebm@gmail.com>
Signed-off-by: Behan Webster <behanw@converseincode.com>
---
 include/linux/moduleparam.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index c3eb102..5ce1f67 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -346,6 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
 /* The macros to do compile-time type checking stolen from Jakub
    Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
 #define __param_check(name, p, type) \
+	static inline type *__check_##name(void) __attribute__ ((unused)); \
 	static inline type *__check_##name(void) { return(p); }
 
 extern struct kernel_param_ops param_ops_byte;
-- 
1.8.3.2

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[PaX Team]

On 7 Mar 2014 at 11:08, behanw@converseincode.com wrote:

> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index c3eb102..5ce1f67 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -346,6 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
>  /* The macros to do compile-time type checking stolen from Jakub
>     Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
>  #define __param_check(name, p, type) \
> +	static inline type *__check_##name(void) __attribute__ ((unused)); \
>  	static inline type *__check_##name(void) { return(p); }

why can't you have the attr on the definition itself:

static inline __unused type *__check_##name(void) { return(p); }

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[Behan Webster]

On 03/07/14 14:56, PaX Team wrote:
> On 7 Mar 2014 at 11:08, behanw@converseincode.com wrote:
>
>> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>> index c3eb102..5ce1f67 100644
>> --- a/include/linux/moduleparam.h
>> +++ b/include/linux/moduleparam.h
>> @@ -346,6 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
>>   /* The macros to do compile-time type checking stolen from Jakub
>>      Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
>>   #define __param_check(name, p, type) \
>> +	static inline type *__check_##name(void) __attribute__ ((unused)); \
>>   	static inline type *__check_##name(void) { return(p); }
> why can't you have the attr on the definition itself:
>
> static inline __unused type *__check_##name(void) { return(p); }
>
>
>
"__unused" isn't defined anywhere I can find, except in:

src/linux/drivers/net/ethernet/amd/declance.c:#define __unused 
__attribute__ ((unused))

But this does work.

-       static inline type *__check_##name(void) { return(p); }
+       static inline __attribute__((unused)) \
+               type *__check_##name(void) { return(p); }

If this is preferred, I will resubmit.

Behan

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[PaX Team]

On 7 Mar 2014 at 16:52, Behan Webster wrote:

> On 03/07/14 14:56, PaX Team wrote:
> > why can't you have the attr on the definition itself:
> >
> > static inline __unused type *__check_##name(void) { return(p); }
> >
> >
> >
> "__unused" isn't defined anywhere I can find, except in:
> 
> src/linux/drivers/net/ethernet/amd/declance.c:#define __unused 
> __attribute__ ((unused))

oh i forgot about that, you'd of course place this #define
into the appropriate compiler*.h files so that other code
can make use of it in the future as well ;).

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[Stephen Boyd]

On 03/07/14 17:25, PaX Team wrote:
> On 7 Mar 2014 at 16:52, Behan Webster wrote:
>
>> On 03/07/14 14:56, PaX Team wrote:
>>> why can't you have the attr on the definition itself:
>>>
>>> static inline __unused type *__check_##name(void) { return(p); }
>>>
>>>
>>>
>> "__unused" isn't defined anywhere I can find, except in:
>>
>> src/linux/drivers/net/ethernet/amd/declance.c:#define __unused 
>> __attribute__ ((unused))
> oh i forgot about that, you'd of course place this #define
> into the appropriate compiler*.h files so that other code
> can make use of it in the future as well ;).
>

It looks like it already exists, __always_unused.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v2] module: LLVMLinux: Remove unused function warning from __param_check macro

[behanw]

From: Mark Charlebois <charlebm@gmail.com>

This code makes a compile time type check that is optimized away. Clang
complains that it generates an unused function.

I believe GCC won't complain for a static inline fuction but would if it
was just a static function.

Adding the unused attribute to the function definition removes the warning.

This code works for both gcc and clang.

Signed-off-by: Mark Charlebois <charlebm@gmail.com>
Signed-off-by: Behan Webster <behanw@converseincode.com>
---
 include/linux/moduleparam.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index c3eb102..7f2a726 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -346,7 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
 /* The macros to do compile-time type checking stolen from Jakub
    Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
 #define __param_check(name, p, type) \
-	static inline type *__check_##name(void) { return(p); }
+	static inline __always_unused type *__check_##name(void) { return(p); }
 
 extern struct kernel_param_ops param_ops_byte;
 extern int param_set_byte(const char *val, const struct kernel_param *kp);
-- 
1.8.3.2

Re: [PATCH v2] module: LLVMLinux: Remove unused function warning from __param_check macro

[Joe Perches]

On Fri, 2014-03-07 at 18:10 -0800, behanw@converseincode.com wrote:
> This code makes a compile time type check that is optimized away. Clang
> complains that it generates an unused function.
[]
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
[]
> @@ -346,7 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
>  /* The macros to do compile-time type checking stolen from Jakub
>     Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
>  #define __param_check(name, p, type) \
> -	static inline type *__check_##name(void) { return(p); }
> +	static inline __always_unused type *__check_##name(void) { return(p); }

Perhaps __maybe_unused ?

Re: [PATCH v2] module: LLVMLinux: Remove unused function warning from __param_check macro

[Behan Webster]

On 03/07/14 18:17, Joe Perches wrote:
> On Fri, 2014-03-07 at 18:10 -0800, behanw@converseincode.com wrote:
>> This code makes a compile time type check that is optimized away. Clang
>> complains that it generates an unused function.
> []
>> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> []
>> @@ -346,7 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
>>   /* The macros to do compile-time type checking stolen from Jakub
>>      Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
>>   #define __param_check(name, p, type) \
>> -	static inline type *__check_##name(void) { return(p); }
>> +	static inline __always_unused type *__check_##name(void) { return(p); }
> Perhaps __maybe_unused ?
I thought about that (and even tested with __maybe_unused), but I 
*think* they are always unused, except at compile time (see comment 
above). Though I could be wrong.

I'm certainly okay with __maybe_unused if that is preferable.

Behan

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[Rusty Russell]

behanw@converseincode.com writes:
> From: Mark Charlebois <charlebm@gmail.com>
>
> This code makes a compile time type check that is optimized away. Clang
> complains that it generates an unused function.
>
> I believe GCC won't complain for a static inline fuction but would if it
> was just a static function.
>
> Adding the unused attribute to the function declaration removes the warning.
>
> This code works for both gcc and clang.
>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Behan Webster <behanw@converseincode.com>

Please include the actual warning clang spits out.  That helps because
(1) I know what you're referring to, and
(2) it helps others if they are later googling for the error.

I don't have any huge objections to this patch (__always_unused) though.

Thanks,
Rusty.

Re: [PATCH] module: LLVMLinux: Remove unused function warning from __param_check macro

[Behan Webster]

On 03/10/14 23:11, Rusty Russell wrote:
> behanw@converseincode.com writes:
>> From: Mark Charlebois <charlebm@gmail.com>
>>
>> This code makes a compile time type check that is optimized away. Clang
>> complains that it generates an unused function.
>>
>> I believe GCC won't complain for a static inline fuction but would if it
>> was just a static function.
>>
>> Adding the unused attribute to the function declaration removes the warning.
>>
>> This code works for both gcc and clang.
>>
>> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
>> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Please include the actual warning clang spits out.  That helps because
> (1) I know what you're referring to, and
> (2) it helps others if they are later googling for the error.
Nice! Will fix.

> I don't have any huge objections to this patch (__always_unused) though.
Already in the posted v2 patch.

However I will post a v3 with your other suggested changes to the commit 
message.

Thanks,

Behan

-- 
Behan Webster
behanw@converseincode.com

[PATCH v3] module: LLVMLinux: Remove unused function warning from __param_check macro

[behanw]

From: Mark Charlebois <charlebm@gmail.com>

This code makes a compile time type check that is optimized away. Clang
complains that it generates an unused function:

linux/kernel/panic.c:471:1: warning: unused function '__check_panic'
      [-Wunused-function]
core_param(panic, panic_timeout, int, 0644);
^
linux/moduleparam.h:283:2: note: expanded from macro
      'core_param'
        param_check_##type(name, &(var));                               \
        ^
<scratch space>:87:1: note: expanded from here
param_check_int
^
linux/moduleparam.h:369:34: note: expanded from macro
      'param_check_int'
#define param_check_int(name, p) __param_check(name, p, int)
                                 ^
linux/moduleparam.h:349:22: note: expanded from macro
      '__param_check'
        static inline type *__check_##name(void) { return(p); }
                            ^
<scratch space>:88:1: note: expanded from here
__check_panic

GCC won't complain for a static inline function but would if it was just
a static function.

Adding the unused attribute to the function declaration removes the warning.
Per request from Rusty Russell it is marked as __always_unused as the code
is meant to be optimized away.

This code works for both GCC and clang.

Signed-off-by: Mark Charlebois <charlebm@gmail.com>
Signed-off-by: Behan Webster <behanw@converseincode.com>
---
 include/linux/moduleparam.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index c3eb102..175f699 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -346,7 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
 /* The macros to do compile-time type checking stolen from Jakub
    Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
 #define __param_check(name, p, type) \
-	static inline type *__check_##name(void) { return(p); }
+	static inline type __always_unused *__check_##name(void) { return(p); }
 
 extern struct kernel_param_ops param_ops_byte;
 extern int param_set_byte(const char *val, const struct kernel_param *kp);
-- 
1.8.3.2

Re: [PATCH v3] module: LLVMLinux: Remove unused function warning from __param_check macro

[Rusty Russell]

behanw@converseincode.com writes:
> From: Mark Charlebois <charlebm@gmail.com>
>
> This code makes a compile time type check that is optimized away. Clang
> complains that it generates an unused function:

Thanks, applied.

Cheers,
Rusty.

>
> linux/kernel/panic.c:471:1: warning: unused function '__check_panic'
>       [-Wunused-function]
> core_param(panic, panic_timeout, int, 0644);
> ^
> linux/moduleparam.h:283:2: note: expanded from macro
>       'core_param'
>         param_check_##type(name, &(var));                               \
>         ^
> <scratch space>:87:1: note: expanded from here
> param_check_int
> ^
> linux/moduleparam.h:369:34: note: expanded from macro
>       'param_check_int'
> #define param_check_int(name, p) __param_check(name, p, int)
>                                  ^
> linux/moduleparam.h:349:22: note: expanded from macro
>       '__param_check'
>         static inline type *__check_##name(void) { return(p); }
>                             ^
> <scratch space>:88:1: note: expanded from here
> __check_panic
>
> GCC won't complain for a static inline function but would if it was just
> a static function.
>
> Adding the unused attribute to the function declaration removes the warning.
> Per request from Rusty Russell it is marked as __always_unused as the code
> is meant to be optimized away.
>
> This code works for both GCC and clang.
>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> ---
>  include/linux/moduleparam.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index c3eb102..175f699 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -346,7 +346,7 @@ static inline void destroy_params(const struct kernel_param *params,
>  /* The macros to do compile-time type checking stolen from Jakub
>     Jelinek, who IIRC came up with this idea for the 2.4 module init code. */
>  #define __param_check(name, p, type) \
> -	static inline type *__check_##name(void) { return(p); }
> +	static inline type __always_unused *__check_##name(void) { return(p); }
>  
>  extern struct kernel_param_ops param_ops_byte;
>  extern int param_set_byte(const char *val, const struct kernel_param *kp);
> -- 
> 1.8.3.2