Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1633440 > unrolled thread

Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-04-30 16:10 +0200
    Re: [PATCH 2/2] kbuild: clang: Disable the  'duplicate-decl-specifier' warning Matthias Kaehlcke <mka@chromium.org> - 2017-05-04 22:00 +0200
      Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-05-08 10:30 +0200

#1633440 — Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning

Hi Matthias,


2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> clang generates plenty of these warnings in different parts of the code.
> They are mostly caused by container_of() and other macros which declare
> a "const <type> *" variable for their internal use which triggers a
> "duplicate 'const' specifier" warning if the <type> is already const
> qualified.
>
> Wording-mostly-from: Michael Davidson <md@google.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>



I think container_of() can be more simple,
dropping the 'const'.

The following patch worked for me.


diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c26dc3..d53672b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -846,11 +846,9 @@ static inline void ftrace_dump(enum
ftrace_dump_mode oops_dump_mode) { }
  * @ptr:       the pointer to the member.
  * @type:      the type of the container struct this is embedded in.
  * @member:    the name of the member within the struct.
- *
  */
 #define container_of(ptr, type, member) ({                     \
-       const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
-       (type *)( (char *)__mptr - offsetof(type,member) );})
+       (type *)((void *)(ptr) - offsetof(type, member));})

 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD



For also this one,
I'd like to try to fix the code rather than hiding warnings.

(We may end up with applying your patch after all,
but I'd like to think about it for now.)



-- 
Best Regards
Masahiro Yamada

[toc] | [next] | [standalone]

#1636000 — Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning

Hi Masahiro,

El Sun, Apr 30, 2017 at 11:01:11PM +0900 Masahiro Yamada ha dit:

> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> > clang generates plenty of these warnings in different parts of the code.
> > They are mostly caused by container_of() and other macros which declare
> > a "const <type> *" variable for their internal use which triggers a
> > "duplicate 'const' specifier" warning if the <type> is already const
> > qualified.
> >
> > Wording-mostly-from: Michael Davidson <md@google.com>
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> 
> 
> 
> I think container_of() can be more simple,
> dropping the 'const'.
> 
> The following patch worked for me.
> 
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 4c26dc3..d53672b 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -846,11 +846,9 @@ static inline void ftrace_dump(enum
> ftrace_dump_mode oops_dump_mode) { }
>   * @ptr:       the pointer to the member.
>   * @type:      the type of the container struct this is embedded in.
>   * @member:    the name of the member within the struct.
> - *
>   */
>  #define container_of(ptr, type, member) ({                     \
> -       const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
> -       (type *)( (char *)__mptr - offsetof(type,member) );})
> +       (type *)((void *)(ptr) - offsetof(type, member));})
> 
>  /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD

Thanks, this eliminates indeed a huge amount of these warnings.

The other big source of warnings is MODULE_DEVICE_TABLE which declares
a const alias of a type that in most cases is already const. One possible
solution would be to remove the 'additional' const qualifier, which
might leave some tables non-const. Another option could be some
hackery to suppress the warning just for the macro. Not sure if any of
this would be acceptable.

> For also this one,
> I'd like to try to fix the code rather than hiding warnings.

I totally agree with the general approach. My clang kernel builds
started with plenty of warnings disabled. I fixed the code for most of
them until I was left with these two extremely noisy ones, for which I
didn't see a clear path.

Cheers

Matthias

[toc] | [prev] | [next] | [standalone]

#1637302

Hi Matthias,


2017-05-05 4:50 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> Hi Masahiro,
>
> El Sun, Apr 30, 2017 at 11:01:11PM +0900 Masahiro Yamada ha dit:
>
>> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
>> > clang generates plenty of these warnings in different parts of the code.
>> > They are mostly caused by container_of() and other macros which declare
>> > a "const <type> *" variable for their internal use which triggers a
>> > "duplicate 'const' specifier" warning if the <type> is already const
>> > qualified.
>> >
>> > Wording-mostly-from: Michael Davidson <md@google.com>
>> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>
>>
>>
>> I think container_of() can be more simple,
>> dropping the 'const'.
>>
>> The following patch worked for me.
>>
>>
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index 4c26dc3..d53672b 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -846,11 +846,9 @@ static inline void ftrace_dump(enum
>> ftrace_dump_mode oops_dump_mode) { }
>>   * @ptr:       the pointer to the member.
>>   * @type:      the type of the container struct this is embedded in.
>>   * @member:    the name of the member within the struct.
>> - *
>>   */
>>  #define container_of(ptr, type, member) ({                     \
>> -       const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>> -       (type *)( (char *)__mptr - offsetof(type,member) );})
>> +       (type *)((void *)(ptr) - offsetof(type, member));})
>>
>>  /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
>>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
>
> Thanks, this eliminates indeed a huge amount of these warnings.


I noticed this solution will drop precious type checking
from container_of().  So, my workaround is not nice.



> The other big source of warnings is MODULE_DEVICE_TABLE which declares
> a const alias of a type that in most cases is already const. One possible
> solution would be to remove the 'additional' const qualifier, which
> might leave some tables non-const. Another option could be some
> hackery to suppress the warning just for the macro. Not sure if any of
> this would be acceptable.

Me neither.


>> For also this one,
>> I'd like to try to fix the code rather than hiding warnings.
>
> I totally agree with the general approach. My clang kernel builds
> started with plenty of warnings disabled. I fixed the code for most of
> them until I was left with these two extremely noisy ones, for which I
> didn't see a clear path.


If we do not come up with a good solution,
I will pick up this patch.





-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web