| Message ID | c4847cd94f86bd98fc563f112e177b317dc21111.1732102551.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | fstests: fix blksize_t printf format warnings across architectures | expand |
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 > >
在 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:
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: > >
在 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: >> >> >
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 >> >>
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:
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(-)