lkml.org 
[lkml]   [2021]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] Enable '-Werror' by default for all kernel builds
From
Date
Am 07.09.21 um 07:32 schrieb Huang Rui:
> On Tue, Sep 07, 2021 at 07:06:04AM +0800, Linus Torvalds wrote:
>> [ Adding some subsystem maintainers ]
>>
>> On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>>> But hopefully most cases are just "people haven't cared enough" and
>>>> easily fixed.
>>> We'll see. For my testbed I disabled the new configuration flag
>>> for the time being because its primary focus is boot tests, and
>>> there won't be any boot tests if images fail to build.
>> Sure, reasonable.
>>
>> I've checked a few of the build errors by doing the appropriate cross
>> compiles, and it doesn't seem bad - but it does seem like we have a
>> number of really pointless long-standing warnings that should have
>> been fixed long ago.
>>
>> For example, looking at sparc64, there are several build errors due to
>> those warnings now being fatal:
>>
>> - drivers/gpu/drm/ttm/ttm_pool.c:386
>>
>> This is a type mismatch error. It looks like __fls() on sparc64
>> returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
>> long'.
>>
>> Oddly enough, the very line after that line does "min_t(unsigned
>> int" to get the types in line.
>>
>> So the immediate reason is "sparc64 is different". But the deeper
>> reason seems to be that ttm_pool.c has odd type assumptions. But that
>> warning should have been fixed long ago, either way.
>>
>> Christian/Huang? I get the feeling that both lines in that file
>> should use the min_t(). Hmm?
>
> Shall we align the return type like __fls() on all the arches?

I think so, yes. IIRC I was a bit surprised that it returns UL on x86. I
mean the maximum possible value here is 63.

>
> But it looks OK for me to change min to min_t() here as well, I can file a
> patch to the update:
>
> - for (order = min(MAX_ORDER - 1UL, __fls(num_pages)); num_pages;
> + for (order = min_t(unsigned int, MAX_ORDER - 1UL, __fls(num_pages)); num_pages;
>
> Christian, what's your opinion?

The "MAX_ORDER - 1UL" can now be changed to "MAX_ORDER - 1", but apart
from that looks good to me.

Thanks,
Christian.

>
> Thanks,
> Ray
>
>> - drivers/input/joystick/analog.c:160
>>
>> #warning Precise timer not defined for this architecture.
>>
>> Unfortunate. I suspect that warning just has to be removed. It has
>> never caused anything to be fixed, it's old to the point of predating
>> the git history. Dmitry?
>>
>> - at least a couple of stringop-overread errors. Attached is a
>> possible for for one of them.
>>
>> The stringop overread is odd, because another one of them is
>>
>> fs/qnx4/dir.c: In function ‘qnx4_readdir’:
>> fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
>> source size 16 [-Werror=stringop-overread]
>> 51 | size = strnlen(de->di_fname, size);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> but I'm not seeing why that one happens on sparc64, but not on arm64
>> or x86-64. There doesn't seem to be anything architecture-specific
>> anywhere in that area.
>>
>> Funky.
>>
>> Davem - attached patch compiles cleanly for me, but I'm not sure it's
>> necessarily the right thing to do, and I didn't check the code
>> generation. Maybe it screws up. Can somebody test on sparc64 and
>> perhaps think about it more than I did?
>>
>> Linus
>> arch/sparc/kernel/mdesc.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
>> index 8e645ddac58e..30f171b7b00c 100644
>> --- a/arch/sparc/kernel/mdesc.c
>> +++ b/arch/sparc/kernel/mdesc.c
>> @@ -39,6 +39,7 @@ struct mdesc_hdr {
>> u32 node_sz; /* node block size */
>> u32 name_sz; /* name block size */
>> u32 data_sz; /* data block size */
>> + char data[];
>> } __attribute__((aligned(16)));
>>
>> struct mdesc_elem {
>> @@ -612,7 +613,7 @@ EXPORT_SYMBOL(mdesc_get_node_info);
>>
>> static struct mdesc_elem *node_block(struct mdesc_hdr *mdesc)
>> {
>> - return (struct mdesc_elem *) (mdesc + 1);
>> + return (struct mdesc_elem *) mdesc->data;
>> }
>>
>> static void *name_block(struct mdesc_hdr *mdesc)

\
 
 \ /
  Last update: 2021-09-07 08:17    [W:0.103 / U:0.828 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site