| Message ID | 662dabdece8e902a1234b84cdc46ffefb107397d.1688396342.git.nabijaczleweli@nabijaczleweli.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] hurd: statvfs: __f_type -> f_type | expand |
On 03/07/23 11:59, наб via Libc-alpha wrote: > This is the only missing part in struct statvfs. > The LSB calls [f]statfs() deprecated, and its weird types are definitely > off-putting. However, its use is required to get f_type. > > Instead, allocate one of the six spares to f_type, > copied directly from struct . > This then becomes a small glibc extension to the standard interface > on Linux and the Hurd, instead of two different interfaces, one of which > is quite odd due to being an ABI type, and there no longer is any reason > to use statfs(). > > The underlying kernel type is a mess, but all architectures agree on u32 > (or more) for the ABI, and all filesystem magicks are 32-bit integers. > > Link: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24szs4raroaz@ogivjbfdaqtb/t/#u > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> > --- > sysdeps/unix/sysv/linux/bits/statvfs.h | 6 ++++-- > sysdeps/unix/sysv/linux/internal_statvfs.c | 2 ++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/bits/statvfs.h b/sysdeps/unix/sysv/linux/bits/statvfs.h > index 8dfb5ce761..cf98460e00 100644 > --- a/sysdeps/unix/sysv/linux/bits/statvfs.h > +++ b/sysdeps/unix/sysv/linux/bits/statvfs.h > @@ -51,7 +51,8 @@ struct statvfs > #endif > unsigned long int f_flag; > unsigned long int f_namemax; > - int __f_spare[6]; > + unsigned int f_type; > + int __f_spare[5]; > }; > > #ifdef __USE_LARGEFILE64 > @@ -71,7 +72,8 @@ struct statvfs64 > #endif > unsigned long int f_flag; > unsigned long int f_namemax; > - int __f_spare[6]; > + unsigned int f_type; > + int __f_spare[5]; > }; > #endif > All ABIs define fstatfs/statfs64 as signed, as the constants in include/uapi/linux/magic.h. Should we do the same here? > diff --git a/sysdeps/unix/sysv/linux/internal_statvfs.c b/sysdeps/unix/sysv/linux/internal_statvfs.c > index 6a1b7b755f..112d3c241a 100644 > --- a/sysdeps/unix/sysv/linux/internal_statvfs.c > +++ b/sysdeps/unix/sysv/linux/internal_statvfs.c > @@ -57,6 +57,7 @@ __internal_statvfs (struct statvfs *buf, const struct statfs *fsbuf) > buf->__f_unused = 0; > #endif > buf->f_namemax = fsbuf->f_namelen; > + buf->f_type = fsbuf->f_type; > memset (buf->__f_spare, '\0', sizeof (buf->__f_spare)); > > /* What remains to do is to fill the fields f_favail and f_flag. */ > @@ -99,6 +100,7 @@ __internal_statvfs64 (struct statvfs64 *buf, const struct statfs64 *fsbuf) > buf->__f_unused = 0; > #endif > buf->f_namemax = fsbuf->f_namelen; > + buf->f_type = fsbuf->f_type; > memset (buf->__f_spare, '\0', sizeof (buf->__f_spare)); > > /* What remains to do is to fill the fields f_favail and f_flag. */
On Fri, Jul 21, 2023 at 10:03:05AM -0300, Adhemerval Zanella Netto wrote: > On 03/07/23 11:59, наб via Libc-alpha wrote: > > diff --git a/sysdeps/unix/sysv/linux/bits/statvfs.h b/sysdeps/unix/sysv/linux/bits/statvfs.h > > index 8dfb5ce761..cf98460e00 100644 > > --- a/sysdeps/unix/sysv/linux/bits/statvfs.h > > +++ b/sysdeps/unix/sysv/linux/bits/statvfs.h > > @@ -51,7 +51,8 @@ struct statvfs > > #endif > > unsigned long int f_flag; > > unsigned long int f_namemax; > > - int __f_spare[6]; > > + unsigned int f_type; > > + int __f_spare[5]; > > }; > > > > #ifdef __USE_LARGEFILE64 > > @@ -71,7 +72,8 @@ struct statvfs64 > > #endif > > unsigned long int f_flag; > > unsigned long int f_namemax; > > - int __f_spare[6]; > > + unsigned int f_type; > > + int __f_spare[5]; > > }; > > #endif > > > All ABIs define fstatfs/statfs64 as signed, as the constants in include/uapi/linux/magic.h. > Should we do the same here? Kernel sources say include/asm-generic/compat.h: compat_int_t f_type; include/linux/statfs.h: long f_type; include/uapi/asm-generic/statfs.h: __statfs_word f_type; include/uapi/asm-generic/statfs.h: __statfs_word f_type; include/uapi/asm-generic/statfs.h: __u32 f_type; and arch/alpha/include/uapi/asm/statfs.h:#define __statfs_word __u32 arch/parisc/include/uapi/asm/statfs.h:#define __statfs_word long include/uapi/asm-generic/statfs.h:#ifndef __statfs_word include/uapi/asm-generic/statfs.h:#define __statfs_word __kernel_long_t include/uapi/asm-generic/statfs.h:#define __statfs_word __u32 so it's a damn mess: it looks to me like it's a signed long when it just kinda worked out that way but u32 when actually constructing this ABI for new arches(?). I opted for an u32 because the raw constants represent "4 bytes of data", not "4-byte number"; this is also consistent with the hurd. That said, $ man fstatfs | grep -i "$(printf '%08x\n' $(man fstatfs | grep -o '0x[^ ]*') | grep '^[8-f]')" BPF_FS_MAGIC 0xcafe4a11 BTRFS_SUPER_MAGIC 0x9123683e CIFS_MAGIC_NUMBER 0xff534d42 EFIVARFS_MAGIC 0xde5e81e4 F2FS_SUPER_MAGIC 0xf2f52010 HPFS_SUPER_MAGIC 0xf995e849 HUGETLBFS_MAGIC 0x958458f6 RAMFS_MAGIC 0x858458f6 SELINUX_MAGIC 0xf97cff8c SMB2_MAGIC_NUMBER 0xfe534d42 VXFS_SUPER_MAGIC 0xa501fcf5 XENFS_SUPER_MAGIC 0xabba1974 so a quick test with $ cat fs.cpp #include <type_traits> #include <stdio.h> #include <sys/vfs.h> #include <linux/magic.h> int main() { printf("signed? %d\n", std::is_signed_v<decltype(statfs::f_type)>); printf("sizeof? %zu\n", sizeof(statfs::f_type)); struct statfs sf; fstatfs(0, &sf); printf("sf.f_type as lu=%lu\n", sf.f_type); unsigned int tpu = sf.f_type; int tpd = sf.f_type; printf("sf.f_type as d=%d? %d\n", tpu, tpu == HUGETLBFS_MAGIC); printf("sf.f_type as u=%u? %d\n", tpd, tpd == HUGETLBFS_MAGIC); } $ dpkg --print-architecture; ./fs < /dev/hugepages/ amd64 signed? 1 sizeof? 8 sf.f_type as lu=2508478710 sf.f_type as d=-1786488586? 1 sf.f_type as u=2508478710? 1 $ dpkg --print-architecture; ./fs < /dev/hugepages/ ppc64el signed? 1 sizeof? 8 sf.f_type as lu=2508478710 sf.f_type as d=-1786488586? 1 sf.f_type as u=2508478710? 1 $ dpkg --print-architecture; ./fs < /dev/hugepages/ s390x signed? 0 sizeof? 4 sf.f_type as lu=2240043254 # my vm doesn't have hugetlbfs, so used ramfs sf.f_type as d=-2054924042? 1 sf.f_type as u=2240043254? 1 reveals that there aren't really any issues we run into with signedness here. In C: switch(tpu/tpd/sf.f_type) { case ..._MAGIC: puts("tpu"); } works, and comparing against const unsigned tt = ..._MAGIC; works; comparing against const int tt = ..._MAGIC; doesn't work for sf.f_type. In C++: c++ fs.cpp -o fs fs.cpp:18:27: error: case value evaluates to 2508478710, which cannot be narrowed to type 'int' [-Wc++11-narrowing] 18 | switch(tpd) { case HUGETLBFS_MAGIC: puts("tpd"); } | ^ /usr/include/linux/magic.h:19:26: note: expanded from macro 'HUGETLBFS_MAGIC' 19 | #define HUGETLBFS_MAGIC 0x958458f6 /* some random number */ | ^ 1 error generated. make: *** [<builtin>: fs] Error 1 So this Must be an u32 to behave correctly in C++, we don't lose anything else for making it unsigned, and it's consistent with Hurd. Thoughts?
On 21/07/23 12:34, наб wrote: > On Fri, Jul 21, 2023 at 10:03:05AM -0300, Adhemerval Zanella Netto wrote: >> On 03/07/23 11:59, наб via Libc-alpha wrote: >>> diff --git a/sysdeps/unix/sysv/linux/bits/statvfs.h b/sysdeps/unix/sysv/linux/bits/statvfs.h >>> index 8dfb5ce761..cf98460e00 100644 >>> --- a/sysdeps/unix/sysv/linux/bits/statvfs.h >>> +++ b/sysdeps/unix/sysv/linux/bits/statvfs.h >>> @@ -51,7 +51,8 @@ struct statvfs >>> #endif >>> unsigned long int f_flag; >>> unsigned long int f_namemax; >>> - int __f_spare[6]; >>> + unsigned int f_type; >>> + int __f_spare[5]; >>> }; >>> >>> #ifdef __USE_LARGEFILE64 >>> @@ -71,7 +72,8 @@ struct statvfs64 >>> #endif >>> unsigned long int f_flag; >>> unsigned long int f_namemax; >>> - int __f_spare[6]; >>> + unsigned int f_type; >>> + int __f_spare[5]; >>> }; >>> #endif >>> >> All ABIs define fstatfs/statfs64 as signed, as the constants in include/uapi/linux/magic.h. >> Should we do the same here? > Kernel sources say > include/asm-generic/compat.h: compat_int_t f_type; > include/linux/statfs.h: long f_type; > include/uapi/asm-generic/statfs.h: __statfs_word f_type; > include/uapi/asm-generic/statfs.h: __statfs_word f_type; > include/uapi/asm-generic/statfs.h: __u32 f_type; > and > arch/alpha/include/uapi/asm/statfs.h:#define __statfs_word __u32 > arch/parisc/include/uapi/asm/statfs.h:#define __statfs_word long > include/uapi/asm-generic/statfs.h:#ifndef __statfs_word > include/uapi/asm-generic/statfs.h:#define __statfs_word __kernel_long_t > include/uapi/asm-generic/statfs.h:#define __statfs_word __u32 > so it's a damn mess: it looks to me like it's a signed long when it just > kinda worked out that way but u32 when actually constructing this ABI > for new arches(?). From kernel headers it is 'long' for 64 bits and 'uint32_t' for 32 bits (and off course with a couple of architectures exceptions). > > I opted for an u32 because the raw constants > represent "4 bytes of data", not "4-byte number"; > this is also consistent with the hurd. > > That said, > $ man fstatfs | grep -i "$(printf '%08x\n' $(man fstatfs | grep -o '0x[^ ]*') | grep '^[8-f]')" > BPF_FS_MAGIC 0xcafe4a11 > BTRFS_SUPER_MAGIC 0x9123683e > CIFS_MAGIC_NUMBER 0xff534d42 > EFIVARFS_MAGIC 0xde5e81e4 > F2FS_SUPER_MAGIC 0xf2f52010 > HPFS_SUPER_MAGIC 0xf995e849 > HUGETLBFS_MAGIC 0x958458f6 > RAMFS_MAGIC 0x858458f6 > SELINUX_MAGIC 0xf97cff8c > SMB2_MAGIC_NUMBER 0xfe534d42 > VXFS_SUPER_MAGIC 0xa501fcf5 > XENFS_SUPER_MAGIC 0xabba1974 > so a quick test with > $ cat fs.cpp > #include <type_traits> > #include <stdio.h> > #include <sys/vfs.h> > #include <linux/magic.h> > > int main() { > printf("signed? %d\n", std::is_signed_v<decltype(statfs::f_type)>); > printf("sizeof? %zu\n", sizeof(statfs::f_type)); > struct statfs sf; > fstatfs(0, &sf); > printf("sf.f_type as lu=%lu\n", sf.f_type); > unsigned int tpu = sf.f_type; > int tpd = sf.f_type; > printf("sf.f_type as d=%d? %d\n", tpu, tpu == HUGETLBFS_MAGIC); > printf("sf.f_type as u=%u? %d\n", tpd, tpd == HUGETLBFS_MAGIC); > } > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > amd64 > signed? 1 > sizeof? 8 > sf.f_type as lu=2508478710 > sf.f_type as d=-1786488586? 1 > sf.f_type as u=2508478710? 1 > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > ppc64el > signed? 1 > sizeof? 8 > sf.f_type as lu=2508478710 > sf.f_type as d=-1786488586? 1 > sf.f_type as u=2508478710? 1 > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > s390x > signed? 0 > sizeof? 4 > sf.f_type as lu=2240043254 # my vm doesn't have hugetlbfs, so used ramfs > sf.f_type as d=-2054924042? 1 > sf.f_type as u=2240043254? 1 > reveals that there aren't really any issues we run into with signedness here. > In C: > switch(tpu/tpd/sf.f_type) { case ..._MAGIC: puts("tpu"); } works, > and comparing against const unsigned tt = ..._MAGIC; works; > comparing against const int tt = ..._MAGIC; doesn't work for sf.f_type. > > In C++: > c++ fs.cpp -o fs > fs.cpp:18:27: error: case value evaluates to 2508478710, which cannot be narrowed to type 'int' [-Wc++11-narrowing] > 18 | switch(tpd) { case HUGETLBFS_MAGIC: puts("tpd"); } > | ^ > /usr/include/linux/magic.h:19:26: note: expanded from macro 'HUGETLBFS_MAGIC' > 19 | #define HUGETLBFS_MAGIC 0x958458f6 /* some random number */ > | ^ > 1 error generated. > make: *** [<builtin>: fs] Error 1 > > So this Must be an u32 to behave correctly in C++, > we don't lose anything else for making it unsigned, > and it's consistent with Hurd. > > Thoughts? Fair enough, I think with current kernel ABI status using unsigned would be the best option.
diff --git a/sysdeps/unix/sysv/linux/bits/statvfs.h b/sysdeps/unix/sysv/linux/bits/statvfs.h index 8dfb5ce761..cf98460e00 100644 --- a/sysdeps/unix/sysv/linux/bits/statvfs.h +++ b/sysdeps/unix/sysv/linux/bits/statvfs.h @@ -51,7 +51,8 @@ struct statvfs #endif unsigned long int f_flag; unsigned long int f_namemax; - int __f_spare[6]; + unsigned int f_type; + int __f_spare[5]; }; #ifdef __USE_LARGEFILE64 @@ -71,7 +72,8 @@ struct statvfs64 #endif unsigned long int f_flag; unsigned long int f_namemax; - int __f_spare[6]; + unsigned int f_type; + int __f_spare[5]; }; #endif diff --git a/sysdeps/unix/sysv/linux/internal_statvfs.c b/sysdeps/unix/sysv/linux/internal_statvfs.c index 6a1b7b755f..112d3c241a 100644 --- a/sysdeps/unix/sysv/linux/internal_statvfs.c +++ b/sysdeps/unix/sysv/linux/internal_statvfs.c @@ -57,6 +57,7 @@ __internal_statvfs (struct statvfs *buf, const struct statfs *fsbuf) buf->__f_unused = 0; #endif buf->f_namemax = fsbuf->f_namelen; + buf->f_type = fsbuf->f_type; memset (buf->__f_spare, '\0', sizeof (buf->__f_spare)); /* What remains to do is to fill the fields f_favail and f_flag. */ @@ -99,6 +100,7 @@ __internal_statvfs64 (struct statvfs64 *buf, const struct statfs64 *fsbuf) buf->__f_unused = 0; #endif buf->f_namemax = fsbuf->f_namelen; + buf->f_type = fsbuf->f_type; memset (buf->__f_spare, '\0', sizeof (buf->__f_spare)); /* What remains to do is to fill the fields f_favail and f_flag. */
This is the only missing part in struct statvfs. The LSB calls [f]statfs() deprecated, and its weird types are definitely off-putting. However, its use is required to get f_type. Instead, allocate one of the six spares to f_type, copied directly from struct statfs. This then becomes a small glibc extension to the standard interface on Linux and the Hurd, instead of two different interfaces, one of which is quite odd due to being an ABI type, and there no longer is any reason to use statfs(). The underlying kernel type is a mess, but all architectures agree on u32 (or more) for the ABI, and all filesystem magicks are 32-bit integers. Link: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24szs4raroaz@ogivjbfdaqtb/t/#u Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> --- sysdeps/unix/sysv/linux/bits/statvfs.h | 6 ++++-- sysdeps/unix/sysv/linux/internal_statvfs.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-)