2016-10-17 12:38:17

by Arnd Bergmann

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

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 <[email protected]>
---
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;
--
2.9.0


2016-10-17 13:39:45

by Johannes Thumshirn

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

On Mon, Oct 17, 2016 at 02:35:46PM +0200, 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 <[email protected]>
> ---

Looks good,
Reviewed-by: Johannes Thumshirn <[email protected]>

--
Johannes Thumshirn Storage
[email protected] +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

2016-10-17 15:53:48

by James Smart

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

Thanks

Signed-off-by: James Smart <[email protected]>

-- james


On 10/17/2016 5: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 <[email protected]>
> ---
> 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;

2016-10-17 17:59:52

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

>>>>> "Arnd" == Arnd Bergmann <[email protected]> writes:

Arnd> A recent bugfix introduced a harmless warning in the lpfc driver:
Arnd> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
Arnd> drivers/scsi/lpfc/lpfc_logmsg.h:56:45: error: format '%ld' expects
Arnd> argument of type 'long int', but argument 9 has type 'size_t {aka
Arnd> const unsigned int}' [-Werror=format=]

Arnd> 'size_t' is always the same width as 'long' in the kernel, but the
Arnd> compiler doesn't know that. The %z modifier is what the standard
Arnd> expects to be used here, and this shuts up the warning.

Applied to 4.10/scsi-queue.

--
Martin K. Petersen Oracle Linux Engineering

2016-10-28 21:03:32

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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 <[email protected]>
> ---
> 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

2016-10-28 21:34:21

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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

2016-10-28 21:44:26

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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

2016-10-28 21:49:26

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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

2016-10-28 21:52:19

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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"

2016-10-28 21:58:44

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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.

2016-10-28 22:04:18

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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

2016-10-28 23:20:12

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH] lpfc: use %zd format string for size_t

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