diff mbox

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

Message ID CAK7LNATD-KbP6GtV_j6NMgcPQYRigz5pA8fdos7hQSzQpL8czw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masahiro Yamada April 30, 2017, 2:01 p.m. UTC
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.


 #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.)

Comments

Matthias Kaehlcke May 4, 2017, 7:50 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada May 8, 2017, 8:29 a.m. UTC | #2
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.
diff mbox

Patch

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 */