[PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nick Alcock]

From: Nick Alcock <nick.alcock@oracle.com>

When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
we observe failures of io/tst-getcwd-abspath:

tst-getcwd-abspath.c:42: numeric comparison failure
   left: 75 (0x4b); from: errno
  right: 2 (0x2); from: ENOENT
tst-getcwd-abspath.c:47: numeric comparison failure
   left: 75 (0x4b); from: errno
  right: 2 (0x2); from: ENOENT
error: 2 test failures

Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
had experience with this class of pain in zic before (a patch which I
should perhaps resubmit or combine with this one), the cause is clear:
the getcwd() implementation was calling readdir(), and in glibc that is
the non-LFS implementation so it falls over with just that error if it
sees a single 64-bit inode.

getcwd() is used in the dynamic linker as part of $ORIGIN support, so
the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
getting into it and causing disaster.

While we're at it, fix likely similar errors in ttyname() (determined
by code inspection, since my /dev is a tmpfs with 32-bit indoes: but
one could run glibc on a system with a 64-bit-inode filesystem and
a static /dev, and then this would probably fail too, the same way.)

	* include/dirent.h: Make __readdir64 hidden in rtld too.
        * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
        in libc only.
	* posix/getcwd.c [__GNU_LIBRARY__] (__readdir64): Define.
	(__getcwd): Use dirent64 and __readdir64.
	* sysdeps/posix/ttyname.c (getttyname): Likewise: also ino64_t
	rather than ino_t.
	(ttyname): Use stat64 and fstat64.
	* sysdeps/posix/ttyname_r.c (getttyname_r): Likewise.
	(__ttyname_r): Likewise.
	sysdeps/unix/sysv/linux/tst-ttyname.c (run_chroot_tests): Use
	readdir64.
---
 include/dirent.h                         |  4 +++-
 sysdeps/posix/getcwd.c                   |  5 +++--
 sysdeps/posix/ttyname.c                  | 16 ++++++++--------
 sysdeps/posix/ttyname_r.c                | 14 +++++++-------
 sysdeps/unix/sysv/linux/i386/readdir64.c | 14 ++++++++------
 sysdeps/unix/sysv/linux/tst-ttyname.c    |  4 ++--
 6 files changed, 31 insertions(+), 26 deletions(-)

(Sent from my home address because my work address is not subscribed.)

Tested on 32-bit and 64-bit x86_64; no new failures on 2.27 or master,
and tst-getcwd-abspath is fixed. Quite possibly tst-ttyname is fixed
too, but as noted I haven't been able to verify it.

The ChangeLog incantations around posix/getcwd.c's top-level #defines
(and readdir64.c's, too) are probably wrong: I'm not sure what the
convention is here.

It is quite possible I'm being unidiomatic in include/dirent.h and
sysdeps/unix/sysv/linux/i386/readdir64.c: I'd be happy to switch to
the idiomatic approach if someone could tell me what it is. :)

diff --git a/include/dirent.h b/include/dirent.h
index caaeb0be85..c1d3c6b53f 100644
--- a/include/dirent.h
+++ b/include/dirent.h
@@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
 extern int __closedir (DIR *__dirp) attribute_hidden;
 extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
 extern struct dirent64 *__readdir64 (DIR *__dirp);
-libc_hidden_proto (__readdir64)
+#  if IS_IN (libc) || IS_IN (rtld)
+   hidden_proto (__readdir64)
+#  endif
 extern int __readdir_r (DIR *__dirp, struct dirent *__entry,
 			struct dirent **__result);
 extern int __readdir64_r (DIR *__dirp, struct dirent64 *__entry,
diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c
index b53433a2dc..f547cd6612 100644
--- a/sysdeps/posix/getcwd.c
+++ b/sysdeps/posix/getcwd.c
@@ -194,6 +194,7 @@ extern char *alloca ();
 
 #ifndef __GNU_LIBRARY__
 # define __lstat64	stat64
+# define __readdir64	readdir64
 #endif
 
 #ifndef _LIBC
@@ -366,14 +367,14 @@ __getcwd (char *buf, size_t size)
 	goto lose;
       fd_needs_closing = false;
 
-      struct dirent *d;
+      struct dirent64 *d;
       bool use_d_ino = true;
       while (1)
 	{
 	  /* Clear errno to distinguish EOF from error if readdir returns
 	     NULL.  */
 	  __set_errno (0);
-	  d = __readdir (dirstream);
+	  d = __readdir64 (dirstream);
 	  if (d == NULL)
 	    {
 	      if (errno == 0)
diff --git a/sysdeps/posix/ttyname.c b/sysdeps/posix/ttyname.c
index 3dae5e8411..dab784c7c6 100644
--- a/sysdeps/posix/ttyname.c
+++ b/sysdeps/posix/ttyname.c
@@ -27,20 +27,20 @@
 
 char *__ttyname;
 
-static char *getttyname (int fd, dev_t mydev, ino_t myino,
+static char *getttyname (int fd, dev_t mydev, ino64_t myino,
 			 int save, int *dostat);
 
 
 libc_freeres_ptr (static char *getttyname_name);
 
 static char *
-getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
+getttyname (int fd, dev_t mydev, ino64_t myino, int save, int *dostat)
 {
   static const char dev[] = "/dev";
   static size_t namelen;
   struct stat st;
   DIR *dirstream;
-  struct dirent *d;
+  struct dirent64 *d;
 
   dirstream = __opendir (dev);
   if (dirstream == NULL)
@@ -49,8 +49,8 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
       return NULL;
     }
 
-  while ((d = __readdir (dirstream)) != NULL)
-    if (((ino_t) d->d_fileno == myino || *dostat)
+  while ((d = __readdir64 (dirstream)) != NULL)
+    if (((ino64_t) d->d_fileno == myino || *dostat)
 	&& strcmp (d->d_name, "stdin")
 	&& strcmp (d->d_name, "stdout")
 	&& strcmp (d->d_name, "stderr"))
@@ -76,7 +76,7 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
 #ifdef _STATBUF_ST_RDEV
 	    && S_ISCHR (st.st_mode) && st.st_rdev == mydev
 #else
-	    && (ino_t) d->d_fileno == myino && st.st_dev == mydev
+	    && (ino64_t) d->d_fileno == myino && st.st_dev == mydev
 #endif
 	   )
 	  {
@@ -97,7 +97,7 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
 char *
 ttyname (int fd)
 {
-  struct stat st;
+  struct stat64 st;
   int dostat = 0;
   char *name;
   int save = errno;
@@ -105,7 +105,7 @@ ttyname (int fd)
   if (!__isatty (fd))
     return NULL;
 
-  if (fstat (fd, &st) < 0)
+  if (fstat64 (fd, &st) < 0)
     return NULL;
 
 #ifdef _STATBUF_ST_RDEV
diff --git a/sysdeps/posix/ttyname_r.c b/sysdeps/posix/ttyname_r.c
index 725de7c4fb..4e9b63028a 100644
--- a/sysdeps/posix/ttyname_r.c
+++ b/sysdeps/posix/ttyname_r.c
@@ -32,16 +32,16 @@
 static const char dev[] = "/dev";
 
 static int getttyname_r (int fd, char *buf, size_t buflen,
-			 dev_t mydev, ino_t myino, int save,
+			 dev_t mydev, ino64_t myino, int save,
 			 int *dostat) __THROW;
 
 static int
-getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
+getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino64_t myino,
 	      int save, int *dostat)
 {
   struct stat st;
   DIR *dirstream;
-  struct dirent *d;
+  struct dirent64 *d;
 
   dirstream = __opendir (dev);
   if (dirstream == NULL)
@@ -50,8 +50,8 @@ getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
       return errno;
     }
 
-  while ((d = __readdir (dirstream)) != NULL)
-    if (((ino_t) d->d_fileno == myino || *dostat)
+  while ((d = __readdir64 (dirstream)) != NULL)
+    if (((ino64_t) d->d_fileno == myino || *dostat)
 	&& strcmp (d->d_name, "stdin")
 	&& strcmp (d->d_name, "stdout")
 	&& strcmp (d->d_name, "stderr"))
@@ -74,7 +74,7 @@ getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
 #ifdef _STATBUF_ST_RDEV
 	    && S_ISCHR (st.st_mode) && st.st_rdev == mydev
 #else
-	    && (ino_t) d->d_fileno == myino && st.st_dev == mydev
+	    && (ino64_t) d->d_fileno == myino && st.st_dev == mydev
 #endif
 	   )
 	  {
@@ -121,7 +121,7 @@ __ttyname_r (int fd, char *buf, size_t buflen)
       return ENOTTY;
     }
 
-  if (fstat (fd, &st) < 0)
+  if (fstat64 (fd, &st) < 0)
     return errno;
 
   /* Prepare the result buffer.  */
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
index 42b73023e0..e2981c7f9c 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
@@ -28,19 +28,21 @@
 #undef DIRENT_TYPE
 
 libc_hidden_def (__readdir64)
+#if IS_IN (libc)
 versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
 
-#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
+#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <olddirent.h>
+#  include <olddirent.h>
 
-#define __READDIR attribute_compat_text_section __old_readdir64
-#define __GETDENTS __old_getdents64
-#define DIRENT_TYPE struct __old_dirent64
+#  define __READDIR attribute_compat_text_section __old_readdir64
+#  define __GETDENTS __old_getdents64
+#  define DIRENT_TYPE struct __old_dirent64
 
-#include <sysdeps/posix/readdir.c>
+#  include <sysdeps/posix/readdir.c>
 
 libc_hidden_def (__old_readdir64)
 
 compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1);
 #endif
+#endif
diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
index 35450e1c62..8a402c685e 100644
--- a/sysdeps/unix/sysv/linux/tst-ttyname.c
+++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
@@ -528,8 +528,8 @@ run_chroot_tests (const char *slavename, int slave)
     int ci = 0;
     DIR *dirstream = opendir ("/dev");
     VERIFY (dirstream != NULL);
-    struct dirent *d;
-    while ((d = readdir (dirstream)) != NULL && ci < 3)
+    struct dirent64 *d;
+    while ((d = readdir64 (dirstream)) != NULL && ci < 3)
       {
         if (strcmp (d->d_name, "console1") &&
             strcmp (d->d_name, "console2") &&
-- 
2.16.2.226.gdbca7b3d5

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Adhemerval Zanella]

On 25/02/2018 22:06, Nick Alcock wrote:
> From: Nick Alcock <nick.alcock@oracle.com>
> 
> When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
> we observe failures of io/tst-getcwd-abspath:
> 
> tst-getcwd-abspath.c:42: numeric comparison failure
>    left: 75 (0x4b); from: errno
>   right: 2 (0x2); from: ENOENT
> tst-getcwd-abspath.c:47: numeric comparison failure
>    left: 75 (0x4b); from: errno
>   right: 2 (0x2); from: ENOENT
> error: 2 test failures
> 
> Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
> had experience with this class of pain in zic before (a patch which I
> should perhaps resubmit or combine with this one), the cause is clear:
> the getcwd() implementation was calling readdir(), and in glibc that is
> the non-LFS implementation so it falls over with just that error if it
> sees a single 64-bit inode.

Thanks for checking on it and I see no reason to continue using non-LFS
calls on loader for 32-bits architectures.

I don't see this regression with a i686-linux-gnu build running on a
x86_64 kernel, so it seems the case where the system configuration is
interfering on the testcases. It would be good if we could isolate such
issues, so do you have any information why exactly getdents is failing?

> 
> getcwd() is used in the dynamic linker as part of $ORIGIN support, so
> the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
> getting into it and causing disaster.
> 
> While we're at it, fix likely similar errors in ttyname() (determined
> by code inspection, since my /dev is a tmpfs with 32-bit indoes: but
> one could run glibc on a system with a 64-bit-inode filesystem and
> a static /dev, and then this would probably fail too, the same way.)

I will recommend to spit the ttyname fix on its own patch (and Linux has
its own versio which already uses LFS calls already).

> 
> 	* include/dirent.h: Make __readdir64 hidden in rtld too.
>         * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
>         in libc only.

Indentation seems a bit off here with extra spaces.

> 	* posix/getcwd.c [__GNU_LIBRARY__] (__readdir64): Define.
> 	(__getcwd): Use dirent64 and __readdir64.
> 	* sysdeps/posix/ttyname.c (getttyname): Likewise: also ino64_t
> 	rather than ino_t.
> 	(ttyname): Use stat64 and fstat64.
> 	* sysdeps/posix/ttyname_r.c (getttyname_r): Likewise.
> 	(__ttyname_r): Likewise.
> 	sysdeps/unix/sysv/linux/tst-ttyname.c (run_chroot_tests): Use
> 	readdir64.
> ---
>  include/dirent.h                         |  4 +++-
>  sysdeps/posix/getcwd.c                   |  5 +++--
>  sysdeps/posix/ttyname.c                  | 16 ++++++++--------
>  sysdeps/posix/ttyname_r.c                | 14 +++++++-------
>  sysdeps/unix/sysv/linux/i386/readdir64.c | 14 ++++++++------
>  sysdeps/unix/sysv/linux/tst-ttyname.c    |  4 ++--
>  6 files changed, 31 insertions(+), 26 deletions(-)
> 
> (Sent from my home address because my work address is not subscribed.)
> 
> Tested on 32-bit and 64-bit x86_64; no new failures on 2.27 or master,
> and tst-getcwd-abspath is fixed. Quite possibly tst-ttyname is fixed
> too, but as noted I haven't been able to verify it.
> 
> The ChangeLog incantations around posix/getcwd.c's top-level #defines
> (and readdir64.c's, too) are probably wrong: I'm not sure what the
> convention is here.
> 
> It is quite possible I'm being unidiomatic in include/dirent.h and
> sysdeps/unix/sysv/linux/i386/readdir64.c: I'd be happy to switch to
> the idiomatic approach if someone could tell me what it is. :)

The readdir LFS consolidation to avoid the multiple version for the
different architecture we have is on my backlog for some time.  I think
current approach is fine.

> 
> diff --git a/include/dirent.h b/include/dirent.h
> index caaeb0be85..c1d3c6b53f 100644
> --- a/include/dirent.h
> +++ b/include/dirent.h
> @@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
>  extern int __closedir (DIR *__dirp) attribute_hidden;
>  extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
>  extern struct dirent64 *__readdir64 (DIR *__dirp);
> -libc_hidden_proto (__readdir64)
> +#  if IS_IN (libc) || IS_IN (rtld)
> +   hidden_proto (__readdir64)
> +#  endif

I think you will need a '&& !defined NO_RTLD_HIDDEN' for Hurd (I can't confirm
even with build-many-glibc.py Hurd is missing the thread configuration to
complete the build).

>  extern int __readdir_r (DIR *__dirp, struct dirent *__entry,
>  			struct dirent **__result);
>  extern int __readdir64_r (DIR *__dirp, struct dirent64 *__entry,
> diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c
> index b53433a2dc..f547cd6612 100644
> --- a/sysdeps/posix/getcwd.c
> +++ b/sysdeps/posix/getcwd.c
> @@ -194,6 +194,7 @@ extern char *alloca ();
>  
>  #ifndef __GNU_LIBRARY__
>  # define __lstat64	stat64
> +# define __readdir64	readdir64
>  #endif
>  
>  #ifndef _LIBC
> @@ -366,14 +367,14 @@ __getcwd (char *buf, size_t size)
>  	goto lose;
>        fd_needs_closing = false;
>  
> -      struct dirent *d;
> +      struct dirent64 *d;
>        bool use_d_ino = true;
>        while (1)
>  	{
>  	  /* Clear errno to distinguish EOF from error if readdir returns
>  	     NULL.  */
>  	  __set_errno (0);
> -	  d = __readdir (dirstream);
> +	  d = __readdir64 (dirstream);
>  	  if (d == NULL)
>  	    {
>  	      if (errno == 0)
> diff --git a/sysdeps/posix/ttyname.c b/sysdeps/posix/ttyname.c
> index 3dae5e8411..dab784c7c6 100644
> --- a/sysdeps/posix/ttyname.c
> +++ b/sysdeps/posix/ttyname.c
> @@ -27,20 +27,20 @@
>  
>  char *__ttyname;
>  
> -static char *getttyname (int fd, dev_t mydev, ino_t myino,
> +static char *getttyname (int fd, dev_t mydev, ino64_t myino,
>  			 int save, int *dostat);
>  
>  
>  libc_freeres_ptr (static char *getttyname_name);
>  
>  static char *
> -getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
> +getttyname (int fd, dev_t mydev, ino64_t myino, int save, int *dostat)
>  {
>    static const char dev[] = "/dev";
>    static size_t namelen;
>    struct stat st;
>    DIR *dirstream;
> -  struct dirent *d;
> +  struct dirent64 *d;
>  
>    dirstream = __opendir (dev);
>    if (dirstream == NULL)
> @@ -49,8 +49,8 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
>        return NULL;
>      }
>  
> -  while ((d = __readdir (dirstream)) != NULL)
> -    if (((ino_t) d->d_fileno == myino || *dostat)
> +  while ((d = __readdir64 (dirstream)) != NULL)
> +    if (((ino64_t) d->d_fileno == myino || *dostat)
>  	&& strcmp (d->d_name, "stdin")
>  	&& strcmp (d->d_name, "stdout")
>  	&& strcmp (d->d_name, "stderr"))
> @@ -76,7 +76,7 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
>  #ifdef _STATBUF_ST_RDEV
>  	    && S_ISCHR (st.st_mode) && st.st_rdev == mydev
>  #else
> -	    && (ino_t) d->d_fileno == myino && st.st_dev == mydev
> +	    && (ino64_t) d->d_fileno == myino && st.st_dev == mydev
>  #endif
>  	   )
>  	  {
> @@ -97,7 +97,7 @@ getttyname (int fd, dev_t mydev, ino_t myino, int save, int *dostat)
>  char *
>  ttyname (int fd)
>  {
> -  struct stat st;
> +  struct stat64 st;
>    int dostat = 0;
>    char *name;
>    int save = errno;
> @@ -105,7 +105,7 @@ ttyname (int fd)
>    if (!__isatty (fd))
>      return NULL;
>  
> -  if (fstat (fd, &st) < 0)
> +  if (fstat64 (fd, &st) < 0)
>      return NULL;
>  
>  #ifdef _STATBUF_ST_RDEV
> diff --git a/sysdeps/posix/ttyname_r.c b/sysdeps/posix/ttyname_r.c
> index 725de7c4fb..4e9b63028a 100644
> --- a/sysdeps/posix/ttyname_r.c
> +++ b/sysdeps/posix/ttyname_r.c
> @@ -32,16 +32,16 @@
>  static const char dev[] = "/dev";
>  
>  static int getttyname_r (int fd, char *buf, size_t buflen,
> -			 dev_t mydev, ino_t myino, int save,
> +			 dev_t mydev, ino64_t myino, int save,
>  			 int *dostat) __THROW;
>  
>  static int
> -getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
> +getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino64_t myino,
>  	      int save, int *dostat)
>  {
>    struct stat st;
>    DIR *dirstream;
> -  struct dirent *d;
> +  struct dirent64 *d;
>  
>    dirstream = __opendir (dev);
>    if (dirstream == NULL)
> @@ -50,8 +50,8 @@ getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
>        return errno;
>      }
>  
> -  while ((d = __readdir (dirstream)) != NULL)
> -    if (((ino_t) d->d_fileno == myino || *dostat)
> +  while ((d = __readdir64 (dirstream)) != NULL)
> +    if (((ino64_t) d->d_fileno == myino || *dostat)
>  	&& strcmp (d->d_name, "stdin")
>  	&& strcmp (d->d_name, "stdout")
>  	&& strcmp (d->d_name, "stderr"))
> @@ -74,7 +74,7 @@ getttyname_r (int fd, char *buf, size_t buflen, dev_t mydev, ino_t myino,
>  #ifdef _STATBUF_ST_RDEV
>  	    && S_ISCHR (st.st_mode) && st.st_rdev == mydev
>  #else
> -	    && (ino_t) d->d_fileno == myino && st.st_dev == mydev
> +	    && (ino64_t) d->d_fileno == myino && st.st_dev == mydev
>  #endif
>  	   )
>  	  {
> @@ -121,7 +121,7 @@ __ttyname_r (int fd, char *buf, size_t buflen)
>        return ENOTTY;
>      }
>  
> -  if (fstat (fd, &st) < 0)
> +  if (fstat64 (fd, &st) < 0)
>      return errno;
>  
>    /* Prepare the result buffer.  */
> diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
> index 42b73023e0..e2981c7f9c 100644
> --- a/sysdeps/unix/sysv/linux/i386/readdir64.c
> +++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
> @@ -28,19 +28,21 @@
>  #undef DIRENT_TYPE
>  
>  libc_hidden_def (__readdir64)
> +#if IS_IN (libc)
>  versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
>  
> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
> +#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>  
> -#include <olddirent.h>
> +#  include <olddirent.h>
>  
> -#define __READDIR attribute_compat_text_section __old_readdir64
> -#define __GETDENTS __old_getdents64
> -#define DIRENT_TYPE struct __old_dirent64
> +#  define __READDIR attribute_compat_text_section __old_readdir64
> +#  define __GETDENTS __old_getdents64
> +#  define DIRENT_TYPE struct __old_dirent64
>  
> -#include <sysdeps/posix/readdir.c>
> +#  include <sysdeps/posix/readdir.c>
>  
>  libc_hidden_def (__old_readdir64)
>  
>  compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1);
>  #endif
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
> index 35450e1c62..8a402c685e 100644
> --- a/sysdeps/unix/sysv/linux/tst-ttyname.c
> +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
> @@ -528,8 +528,8 @@ run_chroot_tests (const char *slavename, int slave)
>      int ci = 0;
>      DIR *dirstream = opendir ("/dev");
>      VERIFY (dirstream != NULL);
> -    struct dirent *d;
> -    while ((d = readdir (dirstream)) != NULL && ci < 3)
> +    struct dirent64 *d;
> +    while ((d = readdir64 (dirstream)) != NULL && ci < 3)
>        {
>          if (strcmp (d->d_name, "console1") &&
>              strcmp (d->d_name, "console2") &&
> 

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Joseph Myers]

This seems like a bug that would have been user-visible in releases, so 
needs filing in Bugzilla.  (We have bug 15333 for the various installed 
programs, but nothing for this issue in the libraries.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nix]

On 26 Feb 2018, Joseph Myers verbalised:

> This seems like a bug that would have been user-visible in releases, so 
> needs filing in Bugzilla.  (We have bug 15333 for the various installed 
> programs, but nothing for this issue in the libraries.)

I wasn't sure if a bug I'd only seen with the testsuite but not in real
life merited such a thing.

I'll raise one, probably tomorrow (it's late, I'm tired, and I don't
want to get it wrong :) ).

-- 
NULL && (void)

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nix]

On 26 Feb 2018, Adhemerval Zanella told this:

> On 25/02/2018 22:06, Nick Alcock wrote:
>> From: Nick Alcock <nick.alcock@oracle.com>
>> 
>> When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
>> we observe failures of io/tst-getcwd-abspath:
>> 
>> tst-getcwd-abspath.c:42: numeric comparison failure
>>    left: 75 (0x4b); from: errno
>>   right: 2 (0x2); from: ENOENT
>> tst-getcwd-abspath.c:47: numeric comparison failure
>>    left: 75 (0x4b); from: errno
>>   right: 2 (0x2); from: ENOENT
>> error: 2 test failures
>> 
>> Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
>> had experience with this class of pain in zic before (a patch which I
>> should perhaps resubmit or combine with this one), the cause is clear:
>> the getcwd() implementation was calling readdir(), and in glibc that is
>> the non-LFS implementation so it falls over with just that error if it
>> sees a single 64-bit inode.
>
> Thanks for checking on it and I see no reason to continue using non-LFS
> calls on loader for 32-bits architectures.

Neither do I. There are quite a lot of non-LFS calls elsewhere (I have
another patch that purges a bunch of them from other tests, for
instance, but they're even less related to this bug than the ttyname
stuff in here was).

> I don't see this regression with a i686-linux-gnu build running on a
> x86_64 kernel, so it seems the case where the system configuration is
> interfering on the testcases. It would be good if we could isolate such
> issues, so do you have any information why exactly getdents is failing?

You need a filesystem on which inode numbers are routinely > 2^32, for
instance a >1TiB XFS filesystem mounted without the
(performance-destroying) inode32 option. On such filesystems, getdents()
(but not getdents64()) will return -EOVERFLOW if any inode number in the
set to be returned will not fit in the 32-bit space of an ino_t. (See
the various references to -EOVERFLOW in fs/readdir.c, which handle one
instance or another of this.)

This happens on xfs because it uses physical disk block index as an
inode number: ext4 etc, which use more conventional inode tables, always
has sub-32-bit inode numbers except in the largest of filesystems and so
will not reveal this problem. But XFS is getting more common these days,
and big filesystems are too... you'll also see it on NFSv4 mounts to
such a kernel (and if youre using NFS, you'll be using NFSv4: NFSv3
mounts will generally not work right because the protocol cannot handle
64-bit inode numbers).

>> getcwd() is used in the dynamic linker as part of $ORIGIN support, so
>> the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
>> getting into it and causing disaster.
>> 
>> While we're at it, fix likely similar errors in ttyname() (determined
>> by code inspection, since my /dev is a tmpfs with 32-bit indoes: but
>> one could run glibc on a system with a 64-bit-inode filesystem and
>> a static /dev, and then this would probably fail too, the same way.)
>
> I will recommend to spit the ttyname fix on its own patch (and Linux has
> its own versio which already uses LFS calls already).

Oh, does it? I missed that, The ttyname part may be entirely unnecessary
then. I'll just drop it for now: if it belongs anywhere it's not here,
and I have no proof it causes any trouble at all (unlike getcwd).

>> 
>> 	* include/dirent.h: Make __readdir64 hidden in rtld too.
>>         * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
>>         in libc only.
>
> Indentation seems a bit off here with extra spaces.

That's what happens when you forget an M-x tabify. :/

>> It is quite possible I'm being unidiomatic in include/dirent.h and
>> sysdeps/unix/sysv/linux/i386/readdir64.c: I'd be happy to switch to
>> the idiomatic approach if someone could tell me what it is. :)
>
> The readdir LFS consolidation to avoid the multiple version for the
> different architecture we have is on my backlog for some time.  I think
> current approach is fine.

Thanks!

>> diff --git a/include/dirent.h b/include/dirent.h
>> index caaeb0be85..c1d3c6b53f 100644
>> --- a/include/dirent.h
>> +++ b/include/dirent.h
>> @@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
>>  extern int __closedir (DIR *__dirp) attribute_hidden;
>>  extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
>>  extern struct dirent64 *__readdir64 (DIR *__dirp);
>> -libc_hidden_proto (__readdir64)
>> +#  if IS_IN (libc) || IS_IN (rtld)
>> +   hidden_proto (__readdir64)
>> +#  endif
>
> I think you will need a '&& !defined NO_RTLD_HIDDEN' for Hurd (I can't confirm
> even with build-many-glibc.py Hurd is missing the thread configuration to
> complete the build).

It does look like it, from other uses. I'll add that, drop the ttyname
stuff for now, open a bug, and resubmit.

-- 
NULL && (void)

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nick Alcock]

On 26 Feb 2018, Nix said:

> I'll raise one, probably tomorrow (it's late, I'm tired, and I don't
> want to get it wrong :) ).

Nah, I can't sleep with work outstanding! (or something like that).

This is now #22899 (taken). I have a rather better explanation there,
too, including precisely when it goes wrong (though the fact that it
depends on directory search order makes it hard to reliably reproduce
unless you have lots of > 2^32 inodes in your disconnected subtree).

I'd say that if you ranked bugs by their likely impact, this one is
waaay down there in unimportant territory (you need a 32-bit libc and a
big fs with 64-bit inodes *and* a disconnected subtree). But hey, let's
fix it anyway.

I'll have another patch posted in a few minutes.

Re: [PATCH v2] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nick Alcock]

Here's the latest patch round, with your review comments incorporated,
and retested on trunk. Thanks for the review!

On 26 Feb 2018, Adhemerval Zanella said:

> Thanks for checking on it and I see no reason to continue using non-LFS
> calls on loader for 32-bits architectures.

I'm sure there are more. I might do a big audit for all remaining
external references to non-LFS stuff from stuff that is not itself a
non-LFS implementation, and squash them all at once. (But maybe we
should do this differently, by internally defining __readdir() etc to be
__readdir64() as is true when _FILE_OFFSET_BITS=64, and adding an
internal __readdir32() for the non-LFS version? That way you'd have to
do something unusual to call the non-LFS variant, which is surely the
right way around.)

I audited all other readdir64() implementations: no others define any
non-default symbol versions except by #including the i386 one, so I
think we're fine on other arches. The Hurd version calls __dir_readdir()
which is not in glibc so I just have to hope that it doesn't have the
same limitation as Linux getdents(). :)

(I hope scissors lines work here: git am --scissors.)

8<---------------------->8
From: Nick Alcock <nick.alcock@oracle.com>

When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
we observe failures of io/tst-getcwd-abspath:

tst-getcwd-abspath.c:42: numeric comparison failure
   left: 75 (0x4b); from: errno
  right: 2 (0x2); from: ENOENT
tst-getcwd-abspath.c:47: numeric comparison failure
   left: 75 (0x4b); from: errno
  right: 2 (0x2); from: ENOENT
error: 2 test failures

Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
had experience with this class of pain in zic before (a patch which I
should perhaps resubmit or combine with this one), the cause is clear:
the getcwd() implementation was calling readdir() because it was in a
disconnected subtree, and in glibc that is the non-LFS implementation
so it ends up calling getdents(), which falls over with just that
error if it sees a single inode with a value nonrepresentable in 32
bits in the directories it fetches, and it scans all parents of the
current directory so it has a lot of opportunities to hit such an
inode.

getcwd() is used in the dynamic linker as part of $ORIGIN support, so
the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
getting into it and causing disaster.

	[BZ #22899]
	* include/dirent.h: Make __readdir64 hidden in rtld too.
	* sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
	in libc only.
	* posix/getcwd.c [__GNU_LIBRARY__] (__readdir64): Define.
	(__getcwd): Use dirent64 and __readdir64.
---
 include/dirent.h                         |  4 +++-
 sysdeps/posix/getcwd.c                   |  5 +++--
 sysdeps/unix/sysv/linux/i386/readdir64.c | 14 ++++++++------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/include/dirent.h b/include/dirent.h
index caaeb0be85..9ca588a4e3 100644
--- a/include/dirent.h
+++ b/include/dirent.h
@@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
 extern int __closedir (DIR *__dirp) attribute_hidden;
 extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
 extern struct dirent64 *__readdir64 (DIR *__dirp);
-libc_hidden_proto (__readdir64)
+#  if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN)
+   hidden_proto (__readdir64)
+#  endif
 extern int __readdir_r (DIR *__dirp, struct dirent *__entry,
 			struct dirent **__result);
 extern int __readdir64_r (DIR *__dirp, struct dirent64 *__entry,
diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c
index b53433a2dc..f547cd6612 100644
--- a/sysdeps/posix/getcwd.c
+++ b/sysdeps/posix/getcwd.c
@@ -194,6 +194,7 @@ extern char *alloca ();
 
 #ifndef __GNU_LIBRARY__
 # define __lstat64	stat64
+# define __readdir64	readdir64
 #endif
 
 #ifndef _LIBC
@@ -366,14 +367,14 @@ __getcwd (char *buf, size_t size)
 	goto lose;
       fd_needs_closing = false;
 
-      struct dirent *d;
+      struct dirent64 *d;
       bool use_d_ino = true;
       while (1)
 	{
 	  /* Clear errno to distinguish EOF from error if readdir returns
 	     NULL.  */
 	  __set_errno (0);
-	  d = __readdir (dirstream);
+	  d = __readdir64 (dirstream);
 	  if (d == NULL)
 	    {
 	      if (errno == 0)
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
index 42b73023e0..e2981c7f9c 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
@@ -28,19 +28,21 @@
 #undef DIRENT_TYPE
 
 libc_hidden_def (__readdir64)
+#if IS_IN (libc)
 versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
 
-#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
+#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <olddirent.h>
+#  include <olddirent.h>
 
-#define __READDIR attribute_compat_text_section __old_readdir64
-#define __GETDENTS __old_getdents64
-#define DIRENT_TYPE struct __old_dirent64
+#  define __READDIR attribute_compat_text_section __old_readdir64
+#  define __GETDENTS __old_getdents64
+#  define DIRENT_TYPE struct __old_dirent64
 
-#include <sysdeps/posix/readdir.c>
+#  include <sysdeps/posix/readdir.c>
 
 libc_hidden_def (__old_readdir64)
 
 compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1);
 #endif
+#endif
-- 
2.16.2.226.gdbca7b3d5

Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Adhemerval Zanella]

On 26/02/2018 17:23, Nix wrote:
> On 26 Feb 2018, Adhemerval Zanella told this:
> 
>> On 25/02/2018 22:06, Nick Alcock wrote:
>>> From: Nick Alcock <nick.alcock@oracle.com>
>>>
>>> When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
>>> we observe failures of io/tst-getcwd-abspath:
>>>
>>> tst-getcwd-abspath.c:42: numeric comparison failure
>>>    left: 75 (0x4b); from: errno
>>>   right: 2 (0x2); from: ENOENT
>>> tst-getcwd-abspath.c:47: numeric comparison failure
>>>    left: 75 (0x4b); from: errno
>>>   right: 2 (0x2); from: ENOENT
>>> error: 2 test failures
>>>
>>> Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
>>> had experience with this class of pain in zic before (a patch which I
>>> should perhaps resubmit or combine with this one), the cause is clear:
>>> the getcwd() implementation was calling readdir(), and in glibc that is
>>> the non-LFS implementation so it falls over with just that error if it
>>> sees a single 64-bit inode.
>>
>> Thanks for checking on it and I see no reason to continue using non-LFS
>> calls on loader for 32-bits architectures.
> 
> Neither do I. There are quite a lot of non-LFS calls elsewhere (I have
> another patch that purges a bunch of them from other tests, for
> instance, but they're even less related to this bug than the ttyname
> stuff in here was).
> 
>> I don't see this regression with a i686-linux-gnu build running on a
>> x86_64 kernel, so it seems the case where the system configuration is
>> interfering on the testcases. It would be good if we could isolate such
>> issues, so do you have any information why exactly getdents is failing?
> 
> You need a filesystem on which inode numbers are routinely > 2^32, for
> instance a >1TiB XFS filesystem mounted without the
> (performance-destroying) inode32 option. On such filesystems, getdents()
> (but not getdents64()) will return -EOVERFLOW if any inode number in the
> set to be returned will not fit in the 32-bit space of an ino_t. (See
> the various references to -EOVERFLOW in fs/readdir.c, which handle one
> instance or another of this.)
> 
> This happens on xfs because it uses physical disk block index as an
> inode number: ext4 etc, which use more conventional inode tables, always
> has sub-32-bit inode numbers except in the largest of filesystems and so
> will not reveal this problem. But XFS is getting more common these days,
> and big filesystems are too... you'll also see it on NFSv4 mounts to
> such a kernel (and if youre using NFS, you'll be using NFSv4: NFSv3
> mounts will generally not work right because the protocol cannot handle
> 64-bit inode numbers).

Thanks for thoroughly explanation, this explain why I am not seeing this 
issue in any of environments I have access.

> 
>>> getcwd() is used in the dynamic linker as part of $ORIGIN support, so
>>> the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
>>> getting into it and causing disaster.
>>>
>>> While we're at it, fix likely similar errors in ttyname() (determined
>>> by code inspection, since my /dev is a tmpfs with 32-bit indoes: but
>>> one could run glibc on a system with a 64-bit-inode filesystem and
>>> a static /dev, and then this would probably fail too, the same way.)
>>
>> I will recommend to spit the ttyname fix on its own patch (and Linux has
>> its own versio which already uses LFS calls already).
> 
> Oh, does it? I missed that, The ttyname part may be entirely unnecessary
> then. I'll just drop it for now: if it belongs anywhere it's not here,
> and I have no proof it causes any trouble at all (unlike getcwd).

Linux uses sysdeps/unix/sysv/linux/ttyname.c and it does not use any
fallback implementation as getcwd.

> 
>>>
>>> 	* include/dirent.h: Make __readdir64 hidden in rtld too.
>>>         * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
>>>         in libc only.
>>
>> Indentation seems a bit off here with extra spaces.
> 
> That's what happens when you forget an M-x tabify. :/
> 
>>> It is quite possible I'm being unidiomatic in include/dirent.h and
>>> sysdeps/unix/sysv/linux/i386/readdir64.c: I'd be happy to switch to
>>> the idiomatic approach if someone could tell me what it is. :)
>>
>> The readdir LFS consolidation to avoid the multiple version for the
>> different architecture we have is on my backlog for some time.  I think
>> current approach is fine.
> 
> Thanks!
> 
>>> diff --git a/include/dirent.h b/include/dirent.h
>>> index caaeb0be85..c1d3c6b53f 100644
>>> --- a/include/dirent.h
>>> +++ b/include/dirent.h
>>> @@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
>>>  extern int __closedir (DIR *__dirp) attribute_hidden;
>>>  extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
>>>  extern struct dirent64 *__readdir64 (DIR *__dirp);
>>> -libc_hidden_proto (__readdir64)
>>> +#  if IS_IN (libc) || IS_IN (rtld)
>>> +   hidden_proto (__readdir64)
>>> +#  endif
>>
>> I think you will need a '&& !defined NO_RTLD_HIDDEN' for Hurd (I can't confirm
>> even with build-many-glibc.py Hurd is missing the thread configuration to
>> complete the build).
> 
> It does look like it, from other uses. I'll add that, drop the ttyname
> stuff for now, open a bug, and resubmit.
> 

Re: [PATCH v2] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Adhemerval Zanella]

On 26/02/2018 20:38, Nick Alcock wrote:
> Here's the latest patch round, with your review comments incorporated,
> and retested on trunk. Thanks for the review!
> 
> On 26 Feb 2018, Adhemerval Zanella said:
> 
>> Thanks for checking on it and I see no reason to continue using non-LFS
>> calls on loader for 32-bits architectures.
> 
> I'm sure there are more. I might do a big audit for all remaining
> external references to non-LFS stuff from stuff that is not itself a
> non-LFS implementation, and squash them all at once. (But maybe we
> should do this differently, by internally defining __readdir() etc to be
> __readdir64() as is true when _FILE_OFFSET_BITS=64, and adding an
> internal __readdir32() for the non-LFS version? That way you'd have to
> do something unusual to call the non-LFS variant, which is surely the
> right way around.)

Usually the internal naming would be __function64 for LFS and __function
for non-LFS, and recently we started to reuse the __libc_function64 and 
__libc_function for newer symbols.  I would prefer to explicit call 
__function64 instead of redefine it with macros, we usually do it on some
implementation because they came from gnulib and it uses this approach
because of its constraints.

> 
> I audited all other readdir64() implementations: no others define any
> non-default symbol versions except by #including the i386 one, so I
> think we're fine on other arches. The Hurd version calls __dir_readdir()
> which is not in glibc so I just have to hope that it doesn't have the
> same limitation as Linux getdents(). :)

I will try to spend some time today to consolidate the readdir64 Linux
implementation.

> 
> (I hope scissors lines work here: git am --scissors.)
> 
> 8<---------------------->8
> From: Nick Alcock <nick.alcock@oracle.com>
> 
> When compiling a 32-bit glibc on a filesystem with 64-bit inodes,
> we observe failures of io/tst-getcwd-abspath:
> 
> tst-getcwd-abspath.c:42: numeric comparison failure
>    left: 75 (0x4b); from: errno
>   right: 2 (0x2); from: ENOENT
> tst-getcwd-abspath.c:47: numeric comparison failure
>    left: 75 (0x4b); from: errno
>   right: 2 (0x2); from: ENOENT
> error: 2 test failures
> 
> Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having
> had experience with this class of pain in zic before (a patch which I
> should perhaps resubmit or combine with this one), the cause is clear:
> the getcwd() implementation was calling readdir() because it was in a
> disconnected subtree, and in glibc that is the non-LFS implementation
> so it ends up calling getdents(), which falls over with just that
> error if it sees a single inode with a value nonrepresentable in 32
> bits in the directories it fetches, and it scans all parents of the
> current directory so it has a lot of opportunities to hit such an
> inode.
> 
> getcwd() is used in the dynamic linker as part of $ORIGIN support, so
> the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
> getting into it and causing disaster.

We usually add where we actually tested the patch on commit message,
so if you may please add the explanation you wrote before about the
XFS inode limitations along where you actually tested (for instance
'checked on x86_64-linux-gnu').

> 
> 	[BZ #22899]
> 	* include/dirent.h: Make __readdir64 hidden in rtld too.
> 	* sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT
> 	in libc only.
> 	* posix/getcwd.c [__GNU_LIBRARY__] (__readdir64): Define.
> 	(__getcwd): Use dirent64 and __readdir64.

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

> ---
>  include/dirent.h                         |  4 +++-
>  sysdeps/posix/getcwd.c                   |  5 +++--
>  sysdeps/unix/sysv/linux/i386/readdir64.c | 14 ++++++++------
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/include/dirent.h b/include/dirent.h
> index caaeb0be85..9ca588a4e3 100644
> --- a/include/dirent.h
> +++ b/include/dirent.h
> @@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden;
>  extern int __closedir (DIR *__dirp) attribute_hidden;
>  extern struct dirent *__readdir (DIR *__dirp) attribute_hidden;
>  extern struct dirent64 *__readdir64 (DIR *__dirp);
> -libc_hidden_proto (__readdir64)
> +#  if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN)
> +   hidden_proto (__readdir64)
> +#  endif
>  extern int __readdir_r (DIR *__dirp, struct dirent *__entry,
>  			struct dirent **__result);
>  extern int __readdir64_r (DIR *__dirp, struct dirent64 *__entry,

Ok.

> diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c
> index b53433a2dc..f547cd6612 100644
> --- a/sysdeps/posix/getcwd.c
> +++ b/sysdeps/posix/getcwd.c
> @@ -194,6 +194,7 @@ extern char *alloca ();
>  
>  #ifndef __GNU_LIBRARY__
>  # define __lstat64	stat64
> +# define __readdir64	readdir64
>  #endif
>  
>  #ifndef _LIBC
> @@ -366,14 +367,14 @@ __getcwd (char *buf, size_t size)
>  	goto lose;
>        fd_needs_closing = false;
>  
> -      struct dirent *d;
> +      struct dirent64 *d;
>        bool use_d_ino = true;
>        while (1)
>  	{
>  	  /* Clear errno to distinguish EOF from error if readdir returns
>  	     NULL.  */
>  	  __set_errno (0);
> -	  d = __readdir (dirstream);
> +	  d = __readdir64 (dirstream);
>  	  if (d == NULL)
>  	    {
>  	      if (errno == 0)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
> index 42b73023e0..e2981c7f9c 100644
> --- a/sysdeps/unix/sysv/linux/i386/readdir64.c
> +++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
> @@ -28,19 +28,21 @@
>  #undef DIRENT_TYPE
>  
>  libc_hidden_def (__readdir64)
> +#if IS_IN (libc)
>  versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
>  
> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
> +#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)

The indentation is done with one extra space.

>  
> -#include <olddirent.h>
> +#  include <olddirent.h>
>  
> -#define __READDIR attribute_compat_text_section __old_readdir64
> -#define __GETDENTS __old_getdents64
> -#define DIRENT_TYPE struct __old_dirent64
> +#  define __READDIR attribute_compat_text_section __old_readdir64
> +#  define __GETDENTS __old_getdents64
> +#  define DIRENT_TYPE struct __old_dirent64
>  
> -#include <sysdeps/posix/readdir.c>
> +#  include <sysdeps/posix/readdir.c>
>  
>  libc_hidden_def (__old_readdir64)
>  
>  compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1);
>  #endif
> +#endif
> 

Ok with the removal of the extra space.

Re: [PATCH v2] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Nick Alcock]

On 27 Feb 2018, Adhemerval Zanella outgrape:

> On 26/02/2018 20:38, Nick Alcock wrote:
>> getcwd() is used in the dynamic linker as part of $ORIGIN support, so
>> the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols
>> getting into it and causing disaster.
>
> We usually add where we actually tested the patch on commit message,
> so if you may please add the explanation you wrote before about the
> XFS inode limitations along where you actually tested (for instance
> 'checked on x86_64-linux-gnu').

Will add.

>> diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
>> index 42b73023e0..e2981c7f9c 100644
>> --- a/sysdeps/unix/sysv/linux/i386/readdir64.c
>> +++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
>> @@ -28,19 +28,21 @@
>>  #undef DIRENT_TYPE
>>  
>>  libc_hidden_def (__readdir64)
>> +#if IS_IN (libc)
>>  versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
>>  
>> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>> +#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>
> The indentation is done with one extra space.

We are already inside one level of #if here: #ifndef _DIRENT_H at the
top. There is a probable error there: # ifndef _ISOMAC is followed by
no identation change at all, so frankly I have no *idea* what level is
appropriate. Perhaps two, perhaps three, certainly not one.

>>  
>> -#include <olddirent.h>
>> +#  include <olddirent.h>
>>  
>> -#define __READDIR attribute_compat_text_section __old_readdir64
>> -#define __GETDENTS __old_getdents64
>> -#define DIRENT_TYPE struct __old_dirent64
>> +#  define __READDIR attribute_compat_text_section __old_readdir64
>> +#  define __GETDENTS __old_getdents64
>> +#  define DIRENT_TYPE struct __old_dirent64
>>  
>> -#include <sysdeps/posix/readdir.c>
>> +#  include <sysdeps/posix/readdir.c>
>>  
>>  libc_hidden_def (__old_readdir64)
>>  
>>  compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1);
>>  #endif
>> +#endif
>
> Ok with the removal of the extra space.

this is a double-level of #if, too, though the indentation on the
innermost #if itself, and on its corresponding #endif, is wrong: will
fix.

-- 
NULL && (void)

Re: [PATCH v2] Fix 32-bit getcwd() on filesystems with 64-bit inodes.

[Zack Weinberg]

On Tue, Feb 27, 2018 at 11:33 AM, Nick Alcock <nix@esperi.org.uk> wrote:
>>> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>>> +#  if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>>
>> The indentation is done with one extra space.
>
> We are already inside one level of #if here: #ifndef _DIRENT_H at the
> top. There is a probable error there: # ifndef _ISOMAC is followed by
> no identation change at all, so frankly I have no *idea* what level is
> appropriate. Perhaps two, perhaps three, certainly not one.

The policy as I understand it is that every level of #if nesting
should increase preprocessor-directive indentation by one space,
*except* for multiple-include guards (the #ifndef _DIRENT_H).

If the file is internally inconsistent, please fix it up as a separate
commit.  Corrections to indentation (and other corrections that only
change white space) do not require prior review - just post the patch
and then check it in.  You do need to add a ChangeLog entry, but it
just needs to say "* whatever.h: Fix indentation."

zw