[PATCH] fstests: fix blksize_t printf format warnings across architectures

[PATCH] fstests: fix blksize_t printf format warnings across architectures

[Anand Jain]

Fix format string warnings when printing blksize_t values that vary
across architectures. The warning occurs because blksize_t is defined
differently between architectures: aarch64 architectures blksize_t is
int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
as below.

 seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
 'long int', but argument 3 has type 'blksize_t' {aka 'int'}

 attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
 'long int', but argument 3 has type '__blksize_t' {aka 'int'}

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 src/attr_replace_test.c | 2 +-
 src/seek_sanity_test.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
index 1218e7264c8f..5d560a633361 100644
--- a/src/attr_replace_test.c
+++ b/src/attr_replace_test.c
@@ -67,7 +67,7 @@ int main(int argc, char *argv[])
 	if (ret < 0) die();
 	size = sbuf.st_blksize * 3 / 4;
 	if (!size)
-		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
+		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);
 	size = MIN(size, maxsize);
 	value = malloc(size);
 	if (!value)
diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index a61ed3da9a8f..c5930357911f 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
 		offset += pos ? 0 : 1;
 	alloc_size = offset;
 done:
-	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
+	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
 	return 0;
 
 fail:
-- 
2.47.0

Re: [PATCH] fstests: fix blksize_t printf format warnings across architectures

[Darrick J. Wong]

On Wed, Nov 20, 2024 at 07:40:41PM +0800, Anand Jain wrote:
> Fix format string warnings when printing blksize_t values that vary
> across architectures. The warning occurs because blksize_t is defined
> differently between architectures: aarch64 architectures blksize_t is
> int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
> as below.
> 
>  seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
>  'long int', but argument 3 has type 'blksize_t' {aka 'int'}
> 
>  attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
>  'long int', but argument 3 has type '__blksize_t' {aka 'int'}
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

I waded through a whole bunch of glibc typedef and macro crud and
discovered that on x64 it can even be long long.  I think.  There were
so many levels of indirection that I am not certain that my analysis was
correct. :(

However, I don't see any harm in explicitly casting to long.  Nobody has
yet come up with a 8GB fsblock filesystem, so we're ok for now. :P

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  src/attr_replace_test.c | 2 +-
>  src/seek_sanity_test.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 1218e7264c8f..5d560a633361 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
>  	if (ret < 0) die();
>  	size = sbuf.st_blksize * 3 / 4;
>  	if (!size)
> -		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> +		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);
>  	size = MIN(size, maxsize);
>  	value = malloc(size);
>  	if (!value)
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index a61ed3da9a8f..c5930357911f 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
>  		offset += pos ? 0 : 1;
>  	alloc_size = offset;
>  done:
> -	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
> +	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
>  	return 0;
>  
>  fail:
> -- 
> 2.47.0
> 
> 

Re: [PATCH] fstests: fix blksize_t printf format warnings across architectures

[Anand Jain]

On 21/11/24 00:28, Darrick J. Wong wrote:
> On Wed, Nov 20, 2024 at 07:40:41PM +0800, Anand Jain wrote:
>> Fix format string warnings when printing blksize_t values that vary
>> across architectures. The warning occurs because blksize_t is defined
>> differently between architectures: aarch64 architectures blksize_t is
>> int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
>> as below.
>>
>>   seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
>>   'long int', but argument 3 has type 'blksize_t' {aka 'int'}
>>
>>   attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
>>   'long int', but argument 3 has type '__blksize_t' {aka 'int'}
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> I waded through a whole bunch of glibc typedef and macro crud and
> discovered that on x64 it can even be long long.  I think.  There were
> so many levels of indirection that I am not certain that my analysis was
> correct. :(
> 

Per preprocessor, it verifies blksize_t is long int and int on x86-64
and aarch64, respectively.

  gcc -E -P -dD -x c - < <(echo '#include <sys/types.h>') | grep blksize_t

  x86-64
    typedef long int __blksize_t;
    typedef __blksize_t blksize_t;

  aarch64
    typedef int __blksize_t;
    typedef __blksize_t blksize_t;


Thanks, Anand

> However, I don't see any harm in explicitly casting to long.  Nobody has
> yet come up with a 8GB fsblock filesystem, so we're ok for now. :P
> 




> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
>> ---
>>   src/attr_replace_test.c | 2 +-
>>   src/seek_sanity_test.c  | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
>> index 1218e7264c8f..5d560a633361 100644
>> --- a/src/attr_replace_test.c
>> +++ b/src/attr_replace_test.c
>> @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
>>   	if (ret < 0) die();
>>   	size = sbuf.st_blksize * 3 / 4;
>>   	if (!size)
>> -		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
>> +		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);
>>   	size = MIN(size, maxsize);
>>   	value = malloc(size);
>>   	if (!value)
>> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
>> index a61ed3da9a8f..c5930357911f 100644
>> --- a/src/seek_sanity_test.c
>> +++ b/src/seek_sanity_test.c
>> @@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
>>   		offset += pos ? 0 : 1;
>>   	alloc_size = offset;
>>   done:
>> -	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
>> +	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
>>   	return 0;
>>   
>>   fail:
>> -- 
>> 2.47.0
>>
>>

Re: [PATCH] fstests: fix blksize_t printf format warnings across architectures

[Qu Wenruo]

在 2024/11/20 22:10, Anand Jain 写道:
> Fix format string warnings when printing blksize_t values that vary
> across architectures. The warning occurs because blksize_t is defined
> differently between architectures: aarch64 architectures blksize_t is
> int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
> as below.
>
>   seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
>   'long int', but argument 3 has type 'blksize_t' {aka 'int'}
>
>   attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
>   'long int', but argument 3 has type '__blksize_t' {aka 'int'}

Why not just use %zu instead?

Thanks,
Qu

>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   src/attr_replace_test.c | 2 +-
>   src/seek_sanity_test.c  | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 1218e7264c8f..5d560a633361 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
>   	if (ret < 0) die();
>   	size = sbuf.st_blksize * 3 / 4;
>   	if (!size)
> -		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> +		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);
>   	size = MIN(size, maxsize);
>   	value = malloc(size);
>   	if (!value)
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index a61ed3da9a8f..c5930357911f 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
>   		offset += pos ? 0 : 1;
>   	alloc_size = offset;
>   done:
> -	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
> +	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
>   	return 0;
>
>   fail:

Re: [PATCH] fstests: fix blksize_t printf format warnings across architectures

[Darrick J. Wong]

On Thu, Nov 21, 2024 at 08:36:58AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2024/11/20 22:10, Anand Jain 写道:
> > Fix format string warnings when printing blksize_t values that vary
> > across architectures. The warning occurs because blksize_t is defined
> > differently between architectures: aarch64 architectures blksize_t is
> > int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
> > as below.
> > 
> >   seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
> >   'long int', but argument 3 has type 'blksize_t' {aka 'int'}
> > 
> >   attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
> >   'long int', but argument 3 has type '__blksize_t' {aka 'int'}
> 
> Why not just use %zu instead?

From printf(3):

       z      A  following  integer conversion corresponds to a size_t
              or ssize_t argument, or a following n conversion  corre‐
              sponds to a pointer to a size_t argument.

blksize_t is not a ssize_t.

--D

> Thanks,
> Qu
> 
> > 
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > ---
> >   src/attr_replace_test.c | 2 +-
> >   src/seek_sanity_test.c  | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> > index 1218e7264c8f..5d560a633361 100644
> > --- a/src/attr_replace_test.c
> > +++ b/src/attr_replace_test.c
> > @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
> >   	if (ret < 0) die();
> >   	size = sbuf.st_blksize * 3 / 4;
> >   	if (!size)
> > -		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> > +		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);
> >   	size = MIN(size, maxsize);
> >   	value = malloc(size);
> >   	if (!value)
> > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> > index a61ed3da9a8f..c5930357911f 100644
> > --- a/src/seek_sanity_test.c
> > +++ b/src/seek_sanity_test.c
> > @@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
> >   		offset += pos ? 0 : 1;
> >   	alloc_size = offset;
> >   done:
> > -	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
> > +	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
> >   	return 0;
> > 
> >   fail:
> 
> 

Re: [PATCH] fstests: fix blksize_t printf format warnings across architectures

[Qu Wenruo]

在 2024/11/21 08:51, Darrick J. Wong 写道:
> On Thu, Nov 21, 2024 at 08:36:58AM +1030, Qu Wenruo wrote:
>>
>>
>> 在 2024/11/20 22:10, Anand Jain 写道:
>>> Fix format string warnings when printing blksize_t values that vary
>>> across architectures. The warning occurs because blksize_t is defined
>>> differently between architectures: aarch64 architectures blksize_t is
>>> int, on x86-64 it's long-int.  Cast the values to long. Fixes warnings
>>> as below.
>>>
>>>    seek_sanity_test.c:110:45: warning: format '%ld' expects argument of type
>>>    'long int', but argument 3 has type 'blksize_t' {aka 'int'}
>>>
>>>    attr_replace_test.c:70:22: warning: format '%ld' expects argument of type
>>>    'long int', but argument 3 has type '__blksize_t' {aka 'int'}
>>
>> Why not just use %zu instead?
> 
>  From printf(3):
> 
>         z      A  following  integer conversion corresponds to a size_t
>                or ssize_t argument, or a following n conversion  corre‐
>                sponds to a pointer to a size_t argument.
> 
> blksize_t is not a ssize_t.

You're right, it's indeed different and it's more complex than I thought.

blksize_t is __SYSCALL_SLONG_TYPE, which has extra handling on x86_64 
with its x32 mode handling.
Only for x86_64 x32 mode it is __SQUAD_TYPE (depending on wordsize 
again) otherwise it's __SLONGWORD_TYPE (fixed to long int).

Meanwhile ssize_t is __SWORD_TYPE, which is only dependent on word size.
For 32bit word size it's __int64_t, and for 64bit it's long int.

So blksize_t is more weird than ssize_t.

Then force converting to long (int) is fine.

> 
> --D
> 
>> Thanks,
>> Qu
>>
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>    src/attr_replace_test.c | 2 +-
>>>    src/seek_sanity_test.c  | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
>>> index 1218e7264c8f..5d560a633361 100644
>>> --- a/src/attr_replace_test.c
>>> +++ b/src/attr_replace_test.c
>>> @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
>>>    	if (ret < 0) die();
>>>    	size = sbuf.st_blksize * 3 / 4;
>>>    	if (!size)
>>> -		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
>>> +		fail("Invalid st_blksize(%ld)\n", (long)sbuf.st_blksize);

Although for this case, I'd prefer to use "long int" just for the sake 
of consistency.

Otherwise looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

>>>    	size = MIN(size, maxsize);
>>>    	value = malloc(size);
>>>    	if (!value)
>>> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
>>> index a61ed3da9a8f..c5930357911f 100644
>>> --- a/src/seek_sanity_test.c
>>> +++ b/src/seek_sanity_test.c
>>> @@ -107,7 +107,7 @@ static int get_io_sizes(int fd)
>>>    		offset += pos ? 0 : 1;
>>>    	alloc_size = offset;
>>>    done:
>>> -	fprintf(stdout, "Allocation size: %ld\n", alloc_size);
>>> +	fprintf(stdout, "Allocation size: %ld\n", (long)alloc_size);
>>>    	return 0;
>>>
>>>    fail:
>>
>>
>