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

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

[наб]

[-- Attachment #1: Type: text/plain, Size: 1023 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>
---
 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 2/2] linux: statvfs: allocate spare for f_type

[наб]

[-- Attachment #1: Type: text/plain, Size: 2577 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.

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
 
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 1/2] hurd: statvfs: __f_type -> f_type

[Samuel Thibault]

наб via Libc-alpha, le ven. 23 juin 2023 22:36:56 +0200, a ecrit:
> 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>

> ---
>  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

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

[Florian Weimer]

* наб via Libc-alpha:

> 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

I think the caller can check for f_type being zero to check if it has
been written by glibc, so this does not require a new symbol version.
The previous already initializes __f_spare to zero, and none of the
magic values in <linux/magic.h> are zero.

This should perhaps have a NEWS entry, though, and a test case.

Thanks,
Florian

[PATCH 3/2] statvfs: f_type: NEWS & test

[наб]

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

Also fix tst-statvfs so that it actually fails;
as it stood, all it did was return 0 always.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 NEWS             |  5 +++++
 io/tst-statvfs.c | 19 +++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 709ee40e50..fc2392f168 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,11 @@ Major new features:
 * The strlcpy and strlcat functions have been added.  They are derived
   from OpenBSD, and are expected to be added to a future POSIX version.
 
+* 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:
 
 * In the Linux kernel for the hppa/parisc architecture some of the
diff --git a/io/tst-statvfs.c b/io/tst-statvfs.c
index 227c62d7da..4f509faa31 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_VERIFY (statvfs (argv[i], &st) == 0);
+      TEST_VERIFY (statfs (argv[i], &stf) == 0);
+      TEST_VERIFY (st.f_type == stf.f_type);
+      printf ("%s: free: %llu, mandatory: %s\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
-                );
+              );
     }
   return 0;
 }
-- 
2.39.2

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

Re: [PATCH 3/2] statvfs: f_type: NEWS & test

[Florian Weimer]

* наб:

>  /* 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_VERIFY (statvfs (argv[i], &st) == 0);
> +      TEST_VERIFY (statfs (argv[i], &stf) == 0);
> +      TEST_VERIFY (st.f_type == stf.f_type);

These TEST_VERIFYs could use TEST_COMPARE for better diagnostics on
failure.

Thanks,
Florian

[PATCH v2 3/2] statvfs: f_type: NEWS & test

[наб]

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

Also fix tst-statvfs so that it actually fails;
as it stood, all it did was return 0 always.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 NEWS             |  5 +++++
 io/tst-statvfs.c | 19 +++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 709ee40e50..fc2392f168 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,11 @@ Major new features:
 * The strlcpy and strlcat functions have been added.  They are derived
   from OpenBSD, and are expected to be added to a future POSIX version.
 
+* 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:
 
 * In the Linux kernel for the hppa/parisc architecture some of the
diff --git a/io/tst-statvfs.c b/io/tst-statvfs.c
index 227c62d7da..b38ecae466 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\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
-                );
+              );
     }
   return 0;
 }
-- 
2.39.2

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