Re: [PATCH] x86: fix two sparse warnings in early boot string handling

[H. Peter Anvin]

On 02/12/2014 03:14 PM, David Rientjes wrote:
> On Wed, 12 Feb 2014, Paul Gortmaker wrote:
> 
>>> This means there is a strstr() prototype that is visible to 
>>> drivers/firmware/efi/efi-stub-helper.c but fails at linkage because you've 
>>> removed the definition. 
>>
>> Yes, because you suggested removal when you said, in what is
>> now deleted context text:
>>
>>  "I don't see why you can't remove strstr() in 
>>  arch/x86/boot/string.c entirely.  What breaks?"
>>
>> The above answers your question. The eboot.c breaks.  So
>> we can't remove strstr.
>>
> 
> Thanks.
> 
>>> So, again, why would you add a duplicate 
>>> prototype with your patch?
>>
>> I'm sure there is an implicit path to <linux/string.h>
>> which allows eboot.c to see a prototype and hence compile.
>>
> 
> Nope, linux/string.h only declares the prototype when 
> #ifndef __HAVE_ARCH_STRSTR and the 32-bit x86 declaration in 
> include/asm/string_32.h properly does #define __HAVE_ARCH_STRSTR.
> 
> There's also no #include ordering issue here since linux/string.h does 
> #include <asm/string.h> first.
> 
> If you had a real problem here, the build would break.  So I'll renew my 
> original objection: I don't think it's acceptable to add unneeded 
> prototypes because sparse doesn't understand this.
> 

The real answer IMO ought to be that since arch/x86/boot/string.c is now
used separately from boot.h (eboot.c which includes efi-stub-helper.c
does *not* include boot.h) we may have to move those string functions
into a separate header file.

Matt, do you have any opinions here?

I don't think it is appropriate to leave these warnings in the code
"just because".  We have too many live warnings, some of which are real
bugs, and we need to encourage people to address them instead of leaving
them lost in the noise.

	-hpa

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/