unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts
@ 2019-09-18 23:54 Alistair Francis
  2019-09-18 23:54 ` [PATCH v2 2/2] sysdeps/statfs: " Alistair Francis
  2019-09-19 20:33 ` [PATCH v2 1/2] sysdeps/stat: " Joseph Myers
  0 siblings, 2 replies; 5+ messages in thread
From: Alistair Francis @ 2019-09-18 23:54 UTC (permalink / raw)
  To: libc-alpha; +Cc: joseph, alistair.francis, macro, alistair23

On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
defined) we want to update the stat struct to remove the padding as it
isn't required. As we don't have the padding we also need to update the
overflow checker to not access the undefined members.

2019-09-16  Alistair Francis  <alistair.francis@wdc.com>

	* sysdeps/unix/sysv/linux/generic/bits/stat.h: Handle 64-bit ino_t types
	on 32-bit hosts.
	* sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h: Likewise.
---
v2:
 - Fix the #if logic

 sysdeps/unix/sysv/linux/generic/bits/stat.h            | 5 ++++-
 sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/generic/bits/stat.h b/sysdeps/unix/sysv/linux/generic/bits/stat.h
index 62aeea5a88d..acd8e7c79a6 100644
--- a/sysdeps/unix/sysv/linux/generic/bits/stat.h
+++ b/sysdeps/unix/sysv/linux/generic/bits/stat.h
@@ -40,7 +40,10 @@
 /* Versions of the `xmknod' interface.  */
 #define _MKNOD_VER_LINUX	0
 
-#if defined __USE_FILE_OFFSET64
+#if defined(__USE_FILE_OFFSET64) || __INO_T_MATCHES_INO64_T == 1
+# if __INO_T_MATCHES_INO64_T == 1 && __OFF_T_MATCHES_OFF64_T != 1
+#  error "ino_t and off_t must both be the same type"
+# endif
 # define __field64(type, type64, name) type64 name
 #elif __WORDSIZE == 64
 # define __field64(type, type64, name) type name
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h b/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
index 45efcd8fd34..7ae01c189eb 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
@@ -36,12 +36,16 @@ static inline off_t lseek_overflow (loff_t res)
 
 static inline int stat_overflow (struct stat *buf)
 {
+#if __INO_T_MATCHES_INO64_T
+  return 0;
+#else
   if (buf->__st_ino_pad == 0 && buf->__st_size_pad == 0
       && buf->__st_blocks_pad == 0)
     return 0;
 
   __set_errno (EOVERFLOW);
   return -1;
+#endif
 }
 
 /* Note that f_files and f_ffree may validly be a sign-extended -1.  */
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] sysdeps/statfs: Handle 64-bit ino_t types on 32-bit hosts
  2019-09-18 23:54 [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts Alistair Francis
@ 2019-09-18 23:54 ` Alistair Francis
  2019-09-19 20:36   ` Joseph Myers
  2019-09-19 20:33 ` [PATCH v2 1/2] sysdeps/stat: " Joseph Myers
  1 sibling, 1 reply; 5+ messages in thread
From: Alistair Francis @ 2019-09-18 23:54 UTC (permalink / raw)
  To: libc-alpha; +Cc: joseph, alistair.francis, macro, alistair23

On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
defined) we want to update the statfs struct to remove the padding as it
isn't required. As we don't have the padding we also need to update the
overflow checker to not access the undefined members.

2019-09-16  Alistair Francis  <alistair.francis@wdc.com>

	* sysdeps/unix/sysv/linux/generic/bits/statfs.h: Handle 64-bit ino_t
	types on 32-bit hosts.
	* sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h: Likewise.
---
v2:
 - Fix the #if logic

 sysdeps/unix/sysv/linux/generic/bits/statfs.h          | 5 ++++-
 sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/generic/bits/statfs.h b/sysdeps/unix/sysv/linux/generic/bits/statfs.h
index 3472084ade0..eab1439acff 100644
--- a/sysdeps/unix/sysv/linux/generic/bits/statfs.h
+++ b/sysdeps/unix/sysv/linux/generic/bits/statfs.h
@@ -32,7 +32,10 @@
    using __USE_FILE_OFFSET64 only see the low 32 bits of some
    of the fields (the __fsblkcnt_t and __fsfilcnt_t fields).  */
 
-#if defined __USE_FILE_OFFSET64
+#if defined(__USE_FILE_OFFSET64) || __INO_T_MATCHES_INO64_T == 1
+# if __INO_T_MATCHES_INO64_T == 1 && __OFF_T_MATCHES_OFF64_T != 1
+#  error "ino_t and off_t must both be the same type"
+# endif
 # define __field64(type, type64, name) type64 name
 #elif __WORDSIZE == 64
 # define __field64(type, type64, name) type name
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h b/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
index 7ae01c189eb..8ac7a1da4bf 100644
--- a/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
+++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h
@@ -51,6 +51,9 @@ static inline int stat_overflow (struct stat *buf)
 /* Note that f_files and f_ffree may validly be a sign-extended -1.  */
 static inline int statfs_overflow (struct statfs *buf)
 {
+#if __INO_T_MATCHES_INO64_T
+  return 0;
+#else
   if (buf->__f_blocks_pad == 0 && buf->__f_bfree_pad == 0
       && buf->__f_bavail_pad == 0
       && (buf->__f_files_pad == 0
@@ -61,4 +64,5 @@ static inline int statfs_overflow (struct statfs *buf)
 
   __set_errno (EOVERFLOW);
   return -1;
+#endif
 }
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts
  2019-09-18 23:54 [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts Alistair Francis
  2019-09-18 23:54 ` [PATCH v2 2/2] sysdeps/statfs: " Alistair Francis
@ 2019-09-19 20:33 ` Joseph Myers
  1 sibling, 0 replies; 5+ messages in thread
From: Joseph Myers @ 2019-09-19 20:33 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha, macro, alistair23

On Wed, 18 Sep 2019, Alistair Francis wrote:

> On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
> defined) we want to update the stat struct to remove the padding as it
> isn't required. As we don't have the padding we also need to update the
> overflow checker to not access the undefined members.

When posting a patch, please *always* say how it was tested.  Typically 
that would include running the glibc testsuite for at least one 
configuration (but there may be exceptions, e.g. documentation patches).

> -#if defined __USE_FILE_OFFSET64
> +#if defined(__USE_FILE_OFFSET64) || __INO_T_MATCHES_INO64_T == 1
> +# if __INO_T_MATCHES_INO64_T == 1 && __OFF_T_MATCHES_OFF64_T != 1

The rule in glibc is that we compile with -Wundef and do not rely on 
undefined macros being interpreted as 0 in #if.

Both __INO_T_MATCHES_INO64_T and __OFF_T_MATCHES_OFF64_T are in fact 
defined/undefined macros, not macros that might sometimes be defined to 1 
and sometimes to 0.  So testing with == there is problematic; I'd expect 
it to cause -Wundef errors for any existing 32-bit linux/generic 
architecture where those macros are undefined (i.e. csky and nios2).

There are two possible approaches to fixing this.

(a) Do a preliminary patch that changes these macros from defined / 
undefined to always-defined with 1 / 0 values.  This involves updating a 
lot of places that use the macros (mainly using __OFF_T_MATCHES_OFF64_T), 
as well as ensuring that all bits/typesizes.h headers provide appropriate 
definitions.

(b) Change this patch to do defined / undefined tests rather than testing 
the macro values.

Also, when a macro is 0 / 1 I'd expect boolean tests just to test the 
macro as a boolean (so plain "__INO_T_MATCHES_INO64_T" inside #if, not 
"__INO_T_MATCHES_INO64_T == 1").

Also, if using "defined" with (), like a function call there should be a 
space before '('.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] sysdeps/statfs: Handle 64-bit ino_t types on 32-bit hosts
  2019-09-18 23:54 ` [PATCH v2 2/2] sysdeps/statfs: " Alistair Francis
@ 2019-09-19 20:36   ` Joseph Myers
  2019-09-23 23:10     ` Alistair Francis
  0 siblings, 1 reply; 5+ messages in thread
From: Joseph Myers @ 2019-09-19 20:36 UTC (permalink / raw)
  To: Alistair Francis; +Cc: libc-alpha, macro, alistair23

On Wed, 18 Sep 2019, Alistair Francis wrote:

> On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
> defined) we want to update the statfs struct to remove the padding as it
> isn't required. As we don't have the padding we also need to update the
> overflow checker to not access the undefined members.

I don't think this is anything to do with ino_t (or off_t).  It's actually 
about whether fsblkcnt_t matches fsblkcnt64_t and fsfilcnt_t matches 
fsfilcnt64_t.  It would seem cleanest for those to get their own macros 
saying whether those types match.  Or, more simply, a single macro 
__STATFS_MATCHES_STATFS64.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] sysdeps/statfs: Handle 64-bit ino_t types on 32-bit hosts
  2019-09-19 20:36   ` Joseph Myers
@ 2019-09-23 23:10     ` Alistair Francis
  0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2019-09-23 23:10 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Alistair Francis, GNU C Library, Maciej W. Rozycki

On Thu, Sep 19, 2019 at 1:36 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 18 Sep 2019, Alistair Francis wrote:
>
> > On a 32-bit platform with a 64-bit ino_t type (__INO_T_MATCHES_INO64_T
> > defined) we want to update the statfs struct to remove the padding as it
> > isn't required. As we don't have the padding we also need to update the
> > overflow checker to not access the undefined members.
>
> I don't think this is anything to do with ino_t (or off_t).  It's actually
> about whether fsblkcnt_t matches fsblkcnt64_t and fsfilcnt_t matches
> fsfilcnt64_t.  It would seem cleanest for those to get their own macros
> saying whether those types match.  Or, more simply, a single macro
> __STATFS_MATCHES_STATFS64.

That makes sense, I have updated this patch.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-09-23 23:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-18 23:54 [PATCH v2 1/2] sysdeps/stat: Handle 64-bit ino_t types on 32-bit hosts Alistair Francis
2019-09-18 23:54 ` [PATCH v2 2/2] sysdeps/statfs: " Alistair Francis
2019-09-19 20:36   ` Joseph Myers
2019-09-23 23:10     ` Alistair Francis
2019-09-19 20:33 ` [PATCH v2 1/2] sysdeps/stat: " Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).