diff mbox

module: LLVMLinux: Remove unused function warning from __param_check macro

Message ID 1394219327-5591-1-git-send-email-behanw@converseincode.com
State New
Headers show

Commit Message

Behan Webster March 7, 2014, 7:08 p.m. UTC
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(+)

Comments

PaX Team March 7, 2014, 10:56 p.m. UTC | #1
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); }



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Behan Webster March 8, 2014, 12:52 a.m. UTC | #2
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
PaX Team March 8, 2014, 1:25 a.m. UTC | #3
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 ;).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Stephen Boyd March 8, 2014, 1:35 a.m. UTC | #4
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.
Rusty Russell March 11, 2014, 6:11 a.m. UTC | #5
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Behan Webster March 11, 2014, 7 a.m. UTC | #6
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
diff mbox

Patch

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;