[PATCH v5 1/2] hurd: statvfs: __f_type -> f_type

[PATCH v5 1/2] hurd: statvfs: __f_type -> f_type

[наб]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

No further changes needed ([f]statvfs() just cast to struct statfs *
and call [f]statfs()).

Link: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24szs4raroaz@ogivjbfdaqtb/t/#u
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
Clean rebase, no changes in 1/2.

 sysdeps/mach/hurd/bits/statvfs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/mach/hurd/bits/statvfs.h b/sysdeps/mach/hurd/bits/statvfs.h
index 3fbfd93335..a94759ef55 100644
--- a/sysdeps/mach/hurd/bits/statvfs.h
+++ b/sysdeps/mach/hurd/bits/statvfs.h
@@ -29,7 +29,7 @@
 
 struct statvfs
   {
-    unsigned int __f_type;
+    unsigned int f_type;
     unsigned long int f_bsize;
 #ifndef __USE_FILE_OFFSET64
     __fsblkcnt_t f_blocks;
@@ -59,7 +59,7 @@ struct statvfs
 #ifdef __USE_LARGEFILE64
 struct statvfs64
   {
-    unsigned int __f_type;
+    unsigned int f_type;
     unsigned long int f_bsize;
     __fsblkcnt64_t f_blocks;
     __fsblkcnt64_t f_bfree;
-- 
2.39.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 2/2] linux: statvfs: allocate spare for f_type

[наб]

[-- Attachment #1: Type: text/plain, Size: 5090 bytes --]

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.
We don't lose any generality by using u32, and by doing so we both make
the API consistent with the Hurd, and allow C++
  switch(f_type) { case RAMFS_MAGIC: ...; }

Also fix tst-statvfs so that it actually fails;
as it stood, all it did was return 0 always.
Test statfs()' and statvfs()' f_types are the same.

Link: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24szs4raroaz@ogivjbfdaqtb/t/#u
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
Clean rebase, squashed 3/3 into 2/2, added note that we need to use u32
to allow switch/cases in C++, io/tst-statvfs now formats statvfs.f_type.

 NEWS                                       |  5 +++++
 io/tst-statvfs.c                           | 19 +++++++++++--------
 sysdeps/unix/sysv/linux/bits/statvfs.h     |  6 ++++--
 sysdeps/unix/sysv/linux/internal_statvfs.c |  2 ++
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 93f7d9faaa..bf79f39065 100644
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,11 @@ Major new features:
   explicitly enabled, then fortify source is forcibly disabled so to keep
   original behavior unchanged.
 
+* struct statvfs now has an f_type member, equal to the f_type statfs member;
+  on the Hurd this was always available under a reserved name,
+  and under Linux a spare has been allocated: it was always zero
+  in previous versions of glibc, and zero is not a valid result.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * libcrypt is no longer built by default, one may use the --enable-crypt
diff --git a/io/tst-statvfs.c b/io/tst-statvfs.c
index 227c62d7da..f3097ce1a8 100644
--- a/io/tst-statvfs.c
+++ b/io/tst-statvfs.c
@@ -1,5 +1,7 @@
 #include <stdio.h>
+#include <sys/statfs.h>
 #include <sys/statvfs.h>
+#include <support/check.h>
 
 
 /* This test cannot detect many errors.  But it will fail if the
@@ -11,17 +13,18 @@ do_test (int argc, char *argv[])
   for (int i = 1; i < argc; ++i)
     {
       struct statvfs st;
-      if (statvfs (argv[i], &st) != 0)
-        printf ("%s: failed (%m)\n", argv[i]);
-      else
-        printf ("%s: free: %llu, mandatory: %s\n", argv[i],
-                (unsigned long long int) st.f_bfree,
+      struct statfs stf;
+      TEST_COMPARE (statvfs (argv[i], &st), 0);
+      TEST_COMPARE (statfs (argv[i], &stf), 0);
+      TEST_COMPARE (st.f_type, stf.f_type);
+      printf ("%s: free: %llu, mandatory: %s, tp=%x\n", argv[i],
+              (unsigned long long int) st.f_bfree,
 #ifdef ST_MANDLOCK
-                (st.f_flag & ST_MANDLOCK) ? "yes" : "no"
+              (st.f_flag & ST_MANDLOCK) ? "yes" : "no",
 #else
-                "no"
+              "no",
 #endif
-                );
+              st.f_type);
     }
   return 0;
 }
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.  */
-- 
2.39.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/2] linux: statvfs: allocate spare for f_type

[Adhemerval Zanella Netto]

On 23/07/23 12:35, наб 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 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.
> We don't lose any generality by using u32, and by doing so we both make
> the API consistent with the Hurd, and allow C++
>   switch(f_type) { case RAMFS_MAGIC: ...; }
> 
> Also fix tst-statvfs so that it actually fails;
> as it stood, all it did was return 0 always.
> Test statfs()' and statvfs()' f_types are the same.
> 
> Link: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24szs4raroaz@ogivjbfdaqtb/t/#u
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

LGTM for 2.39 (so you will probably need to resend once NEWS is adjusted).

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
> Clean rebase, squashed 3/3 into 2/2, added note that we need to use u32
> to allow switch/cases in C++, io/tst-statvfs now formats statvfs.f_type.
> 
>  NEWS                                       |  5 +++++
>  io/tst-statvfs.c                           | 19 +++++++++++--------
>  sysdeps/unix/sysv/linux/bits/statvfs.h     |  6 ++++--
>  sysdeps/unix/sysv/linux/internal_statvfs.c |  2 ++
>  4 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 93f7d9faaa..bf79f39065 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -54,6 +54,11 @@ Major new features:
>    explicitly enabled, then fortify source is forcibly disabled so to keep
>    original behavior unchanged.
>  
> +* struct statvfs now has an f_type member, equal to the f_type statfs member;
> +  on the Hurd this was always available under a reserved name,
> +  and under Linux a spare has been allocated: it was always zero
> +  in previous versions of glibc, and zero is not a valid result.
> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * libcrypt is no longer built by default, one may use the --enable-crypt
> diff --git a/io/tst-statvfs.c b/io/tst-statvfs.c
> index 227c62d7da..f3097ce1a8 100644
> --- a/io/tst-statvfs.c
> +++ b/io/tst-statvfs.c
> @@ -1,5 +1,7 @@
>  #include <stdio.h>
> +#include <sys/statfs.h>
>  #include <sys/statvfs.h>
> +#include <support/check.h>
>  
>  
>  /* This test cannot detect many errors.  But it will fail if the
> @@ -11,17 +13,18 @@ do_test (int argc, char *argv[])
>    for (int i = 1; i < argc; ++i)
>      {
>        struct statvfs st;
> -      if (statvfs (argv[i], &st) != 0)
> -        printf ("%s: failed (%m)\n", argv[i]);
> -      else
> -        printf ("%s: free: %llu, mandatory: %s\n", argv[i],
> -                (unsigned long long int) st.f_bfree,
> +      struct statfs stf;
> +      TEST_COMPARE (statvfs (argv[i], &st), 0);
> +      TEST_COMPARE (statfs (argv[i], &stf), 0);
> +      TEST_COMPARE (st.f_type, stf.f_type);
> +      printf ("%s: free: %llu, mandatory: %s, tp=%x\n", argv[i],
> +              (unsigned long long int) st.f_bfree,
>  #ifdef ST_MANDLOCK
> -                (st.f_flag & ST_MANDLOCK) ? "yes" : "no"
> +              (st.f_flag & ST_MANDLOCK) ? "yes" : "no",
>  #else
> -                "no"
> +              "no",
>  #endif
> -                );
> +              st.f_type);
>      }
>    return 0;
>  }
> 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.  */