[PATCH] lpfc: use %zd format string for size_t

[PATCH] lpfc: use %zd format string for size_t

[Vineet Gupta]

Hi Arnd,

On 10/17/2016 05:35 AM, Arnd Bergmann wrote:
> A recent bugfix introduced a harmless warning in the lpfc driver:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
> drivers/scsi/lpfc/lpfc_logmsg.h:56:45: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]
> 
> 'size_t' is always the same width as 'long' in the kernel, but the compiler
> doesn't know that. The %z modifier is what the standard expects to be
> used here, and this shuts up the warning.
> 
> Fixes: 679053c651fb ("scsi: lpfc: Fix fw download on SLI-4 FC adapters")
> Signed-off-by: Arnd Bergmann <arnd at arndb.de>
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 7be9b8a7bb19..4776fd85514f 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -10332,7 +10332,7 @@ lpfc_write_firmware(const struct firmware *fw, void *context)
>  	    ftype != LPFC_FILE_TYPE_GROUP || fsize != fw->size) {
>  		lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
>  				"3022 Invalid FW image found. "
> -				"Magic:%x Type:%x ID:%x Size %d %ld\n",
> +				"Magic:%x Type:%x ID:%x Size %d %zd\n",
>  				magic_number, ftype, fid, fsize, fw->size);
>  		rc = -EINVAL;
>  		goto release_out;
> 

I'm trying to use about to be released ARC gcc 6.x with current kernels and see a
flood of warnings due to these legit fixes - i.e.g arc gcc 6.2 complains when it
sees -zx formats.

CC      mm/percpu.o
../mm/percpu.c: In function ?pcpu_alloc?:
../mm/percpu.c:890:14: warning: format ?%zu? expects argument of type ?size_t?,
but argument 4 has type ?unsigned int? [-Wformat=]
   WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",

I'm not sure what is going on since the data type is size_t alright - although
from posix_types.h is

typedef unsigned int __kernel_size_t;
typedef __kernel_size_t size_t;

And this seems to be same for ARC as well as ARM. I tried ARM gcc 6.1 @
https://snapshots.linaro.org/components/toolchain/binaries/6.1-2016.08-rc1/arm-linux-gnueabihf/

which doesn't seem to be complaining.

With V=1, I checked the respective ARM and ARC toggles in play, but nothing
related to this seems to be standing out.

I know this is more of a question to our GNU folks, but was wondering if you had
more insight into it - which you almost always do :-)

Thx,
-Vineet

[PATCH] lpfc: use %zd format string for size_t

[Arnd Bergmann]

On Friday, October 28, 2016 2:03:21 PM CEST Vineet Gupta wrote:
> 
> I'm trying to use about to be released ARC gcc 6.x with current kernels and see a
> flood of warnings due to these legit fixes - i.e.g arc gcc 6.2 complains when it
> sees -zx formats.
> 
> CC      mm/percpu.o
> ../mm/percpu.c: In function ?pcpu_alloc?:
> ../mm/percpu.c:890:14: warning: format ?%zu? expects argument of type ?size_t?,
> but argument 4 has type ?unsigned int? [-Wformat=]
>    WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",
> 
> I'm not sure what is going on since the data type is size_t alright - although
> from posix_types.h is
> 
> typedef unsigned int __kernel_size_t;
> typedef __kernel_size_t size_t;
> 
> And this seems to be same for ARC as well as ARM. I tried ARM gcc 6.1 @
> https://snapshots.linaro.org/components/toolchain/binaries/6.1-2016.08-rc1/arm-linux-gnueabihf/
> 
> which doesn't seem to be complaining.
> 
> With V=1, I checked the respective ARM and ARC toggles in play, but nothing
> related to this seems to be standing out.
> 
> I know this is more of a question to our GNU folks, but was wondering if you had
> more insight into it - which you almost always do 

I've seen the problem you describe before, but I don't remember the
exact details. I think what happened is that the compiler knows
what type size_t is supposed to be, either unsigned int or unsigned
long, regardless of what our kernel headers say it is.

This is configuration specific, and something caused your compiler to
be built assuming that size_t is unsigned long, while the kernel
headers are assuming it should be unsigned int.

You can try overriding __kernel_size_t in your asm/posix_types.h
to define it as unsigned long, or try to build your compiler
to match the kernel headers, but the first step would be to find
out why the compiler changed in the first place, assuming that older
compiler versions were matching the kernel here.

	Arnd

[PATCH] lpfc: use %zd format string for size_t

[Vineet Gupta]

On 10/28/2016 02:33 PM, Arnd Bergmann wrote:
> On Friday, October 28, 2016 2:03:21 PM CEST Vineet Gupta wrote:
>>
>> I'm trying to use about to be released ARC gcc 6.x with current kernels and see a
>> flood of warnings due to these legit fixes - i.e.g arc gcc 6.2 complains when it
>> sees -zx formats.
>>
>> CC      mm/percpu.o
>> ../mm/percpu.c: In function ?pcpu_alloc?:
>> ../mm/percpu.c:890:14: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 4 has type ?unsigned int? [-Wformat=]
>>    WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",
>>
>> I'm not sure what is going on since the data type is size_t alright - although
>> from posix_types.h is
>>
>> typedef unsigned int __kernel_size_t;
>> typedef __kernel_size_t size_t;
>>
>> And this seems to be same for ARC as well as ARM. I tried ARM gcc 6.1 @
>> https://snapshots.linaro.org/components/toolchain/binaries/6.1-2016.08-rc1/arm-linux-gnueabihf/
>>
>> which doesn't seem to be complaining.
>>
>> With V=1, I checked the respective ARM and ARC toggles in play, but nothing
>> related to this seems to be standing out.
>>
>> I know this is more of a question to our GNU folks, but was wondering if you had
>> more insight into it - which you almost always do 
> 
> I've seen the problem you describe before, but I don't remember the
> exact details. I think what happened is that the compiler knows
> what type size_t is supposed to be, either unsigned int or unsigned
> long, regardless of what our kernel headers say it is.
> 
> This is configuration specific, and something caused your compiler to
> be built assuming that size_t is unsigned long, while the kernel
> headers are assuming it should be unsigned int.
> 
> You can try overriding __kernel_size_t in your asm/posix_types.h
> to define it as unsigned long, 


Indeed if I hack include/linux/types.h

-typedef __kernel_size_t                size_t;
+typedef unsigned long          size_t;

then the warning goes away, so gcc is indeed assuming size_t to be unsigned long
and not unsigned int. That helps a lot.

or try to build your compiler
> to match the kernel headers, but the first step would be to find
> out why the compiler changed in the first place, assuming that older
> compiler versions were matching the kernel here.
> 
> 	Arnd

[PATCH] lpfc: use %zd format string for size_t

[Arnd Bergmann]

On Friday, October 28, 2016 2:44:13 PM CEST Vineet Gupta wrote:
> 
> Indeed if I hack include/linux/types.h
> 
> -typedef __kernel_size_t                size_t;
> +typedef unsigned long          size_t;
> 
> then the warning goes away, so gcc is indeed assuming size_t to be unsigned long
> and not unsigned int. That helps a lot.

Ok, just be aware that this will introduce warnings for any
compiler that is built to expect an 'unsigned int size_t'
typedef.

	Arnd

[PATCH] lpfc: use %zd format string for size_t

[Vineet Gupta]

On 10/28/2016 02:44 PM, Vineet Gupta wrote:
> This is configuration specific, and something caused your compiler to
>> be built assuming that size_t is unsigned long, while the kernel
>> headers are assuming it should be unsigned int.

So yes this seems to be target specific gcc thing

for ARC 4.8

#define PTRDIFF_TYPE "int"

ARM

#ifndef PTRDIFF_TYPE
#define PTRDIFF_TYPE (TARGET_AAPCS_BASED ? "int" : "long int")
#endif

ARC gcc 6.2

#undef PTRDIFF_TYPE
#define PTRDIFF_TYPE "long int"

[PATCH] lpfc: use %zd format string for size_t

[Vineet Gupta]

On 10/28/2016 02:52 PM, Vineet Gupta wrote:
> On 10/28/2016 02:44 PM, Vineet Gupta wrote:
>> This is configuration specific, and something caused your compiler to
>>> be built assuming that size_t is unsigned long, while the kernel
>>> headers are assuming it should be unsigned int.
> 
> So yes this seems to be target specific gcc thing
> 
> for ARC 4.8
> 
> #define PTRDIFF_TYPE "int"
> 
> ARM
> 
> #ifndef PTRDIFF_TYPE
> #define PTRDIFF_TYPE (TARGET_AAPCS_BASED ? "int" : "long int")
> #endif
> 
> ARC gcc 6.2
> 
> #undef PTRDIFF_TYPE
> #define PTRDIFF_TYPE "long int"

Actually we need to adjust SIZE_TYPE (unsigned int) and PTRDIFF_TYPE (int) in the
gcc 6.x to fix this issue. And that is exactly what ARC gcc 4.8 have.

[PATCH] lpfc: use %zd format string for size_t

[Arnd Bergmann]

On Friday, October 28, 2016 2:58:33 PM CEST Vineet Gupta wrote:
> On 10/28/2016 02:52 PM, Vineet Gupta wrote:
> > On 10/28/2016 02:44 PM, Vineet Gupta wrote:
> >> This is configuration specific, and something caused your compiler to
> >>> be built assuming that size_t is unsigned long, while the kernel
> >>> headers are assuming it should be unsigned int.
> > 
> > So yes this seems to be target specific gcc thing
> > 
> > for ARC 4.8
> > 
> > #define PTRDIFF_TYPE "int"
> > 
> > ARM
> > 
> > #ifndef PTRDIFF_TYPE
> > #define PTRDIFF_TYPE (TARGET_AAPCS_BASED ? "int" : "long int")
> > #endif
> > 
> > ARC gcc 6.2
> > 
> > #undef PTRDIFF_TYPE
> > #define PTRDIFF_TYPE "long int"
> 
> Actually we need to adjust SIZE_TYPE (unsigned int) and PTRDIFF_TYPE (int) in the
> gcc 6.x to fix this issue. And that is exactly what ARC gcc 4.8 have.

What compiler versions are most commonly used these days?

You should probably stay with the version that most people have
and then update either the compiler or the kernel, whichever
diverges from it.

I see in the gcc git log that the version that had "int" got removed
at some point, and the version that had "unsigned int" was added
later.

	Arnd

[PATCH] lpfc: use %zd format string for size_t

[Vineet Gupta]

On 10/28/2016 03:03 PM, Arnd Bergmann wrote:
> On Friday, October 28, 2016 2:58:33 PM CEST Vineet Gupta wrote:
>> On 10/28/2016 02:52 PM, Vineet Gupta wrote:
>>> On 10/28/2016 02:44 PM, Vineet Gupta wrote:
>>>> This is configuration specific, and something caused your compiler to
>>>>> be built assuming that size_t is unsigned long, while the kernel
>>>>> headers are assuming it should be unsigned int.
>>>
>>> So yes this seems to be target specific gcc thing
>>>
>>> for ARC 4.8
>>>
>>> #define PTRDIFF_TYPE "int"
>>>
>>> ARM
>>>
>>> #ifndef PTRDIFF_TYPE
>>> #define PTRDIFF_TYPE (TARGET_AAPCS_BASED ? "int" : "long int")
>>> #endif
>>>
>>> ARC gcc 6.2
>>>
>>> #undef PTRDIFF_TYPE
>>> #define PTRDIFF_TYPE "long int"
>>
>> Actually we need to adjust SIZE_TYPE (unsigned int) and PTRDIFF_TYPE (int) in the
>> gcc 6.x to fix this issue. And that is exactly what ARC gcc 4.8 have.
> 
> What compiler versions are most commonly used these days?

gcc 4.8 is used in production, but internally we are now moving towards 6.0 (to be
officially released soon)

> You should probably stay with the version that most people have
> and then update either the compiler or the kernel, whichever
> diverges from it.

In this case, the issue is simple - gcc 6.x doesn't behave the same as 4.8 so it
needs fixing.

> I see in the gcc git log that the version that had "int" got removed
> at some point, and the version that had "unsigned int" was added
> later.

The upstream version (per initial port) always had

#define SIZE_TYPE "long unsigned int"

which we fixed out-of-tree for 4.8 and this needs to be fixed now for gcc 6.x in
upstream too.

-Vineet