[v2] io: Reorganize the getcwd implementation

Message ID 20200827192021.2498505-1-adhemerval.zanella@linaro.org
State Superseded
Headers
Series [v2] io: Reorganize the getcwd implementation |

Commit Message

Adhemerval Zanella Aug. 27, 2020, 7:20 p.m. UTC
  Changes from previous version:

  - Do not build the generic implementation for rtld.
  - Fix a missing include on Linux implementation (for PATH_MAX).

---

The generic implementation uses two internal symbols: __getcwd_system
(which might be overriden by the system) and __getcwd_generic (the
generic implementation shared with gnulib).  The Linux implementation
is moved to __getcwd_system and generic POSIX implementation is moved
to __getcwd_generic.

For rtld, the fallback code is not enabled since it would only used
for path larger than PATH_MAX and subsequent open calls would fail
with ENAMETOOLONG.  It allows to simplify the code on _dl_new_object
and remove the NO_ALLOCATION build switch on Linux implementation.

This change aims to make the code sync with gnulib easier and simplify
the Linux override implementation.

The dl-fxstatat64 is not required anymore and adding it explicit issue
a duplicate symbol in libc.so linking.

Hurd still overrides the getcwd altogether and one possibility would
to be move its implementation to __getcwd_system and reimplement the
__getcwd_generic to be a empty one.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 elf/dl-object.c                               |  31 ++--
 include/unistd.h                              |   2 +
 io/Makefile                                   |   2 +-
 sysdeps/posix/getcwd.c => io/getcwd-generic.c |   9 +-
 io/getcwd-system.c                            |  28 ++++
 io/getcwd.c                                   |  16 ++-
 sysdeps/unix/sysv/linux/Makefile              |   3 +-
 sysdeps/unix/sysv/linux/alpha/dl-fxstatat64.c |   1 -
 sysdeps/unix/sysv/linux/dl-fxstatat64.c       |   1 -
 sysdeps/unix/sysv/linux/dl-getcwd.c           |   1 -
 sysdeps/unix/sysv/linux/getcwd-system.c       |  69 +++++++++
 sysdeps/unix/sysv/linux/getcwd.c              | 136 ------------------
 .../sysv/linux/sparc/sparc64/dl-fxstatat64.c  |   1 -
 .../sysv/linux/wordsize-64/dl-fxstatat64.c    |   1 -
 14 files changed, 125 insertions(+), 176 deletions(-)
 rename sysdeps/posix/getcwd.c => io/getcwd-generic.c (98%)
 create mode 100644 io/getcwd-system.c
 delete mode 100644 sysdeps/unix/sysv/linux/alpha/dl-fxstatat64.c
 delete mode 100644 sysdeps/unix/sysv/linux/dl-fxstatat64.c
 delete mode 100644 sysdeps/unix/sysv/linux/dl-getcwd.c
 create mode 100644 sysdeps/unix/sysv/linux/getcwd-system.c
 delete mode 100644 sysdeps/unix/sysv/linux/getcwd.c
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/dl-fxstatat64.c
 delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/dl-fxstatat64.c
  

Comments

Paul Eggert Aug. 27, 2020, 11:44 p.m. UTC | #1
This patch doesn't apply to glibc master. 'git am' says:

Applying: io: Reorganize the getcwd implementation
error: patch failed: sysdeps/posix/getcwd.c:89
error: sysdeps/posix/getcwd.c: patch does not apply
Patch failed at 0001 io: Reorganize the getcwd implementation

and plain 'patch' says:

patching file io/getcwd-generic.c (renamed from sysdeps/posix/getcwd.c)
Hunk #1 FAILED at 89.
Hunk #2 FAILED at 154.
Hunk #3 succeeded at 529 with fuzz 1 (offset 43 lines).
2 out of 3 hunks FAILED -- saving rejects to file io/getcwd-generic.c.rej

Perhaps I should be applying it to some particular commit? Or perhaps you need 
to rebase it or something.
  
Adhemerval Zanella Aug. 31, 2020, 6:27 p.m. UTC | #2
On 27/08/2020 20:44, Paul Eggert wrote:
> This patch doesn't apply to glibc master. 'git am' says:
> 
> Applying: io: Reorganize the getcwd implementation
> error: patch failed: sysdeps/posix/getcwd.c:89
> error: sysdeps/posix/getcwd.c: patch does not apply
> Patch failed at 0001 io: Reorganize the getcwd implementation
> 
> and plain 'patch' says:
> 
> patching file io/getcwd-generic.c (renamed from sysdeps/posix/getcwd.c)
> Hunk #1 FAILED at 89.
> Hunk #2 FAILED at 154.
> Hunk #3 succeeded at 529 with fuzz 1 (offset 43 lines).
> 2 out of 3 hunks FAILED -- saving rejects to file io/getcwd-generic.c.rej
> 
> Perhaps I should be applying it to some particular commit? Or perhaps you need to rebase it or something.

Sorry, I should have make it explicit it is an updated for the 4/4 patch
on the set [1]. I will send a updated version now you updated gnulib side
along with some changes on the linux implementation and probably a fix for
BZ#26545 as well.

[1] https://sourceware.org/pipermail/libc-alpha/2020-August/117296.html
  

Patch

diff --git a/elf/dl-object.c b/elf/dl-object.c
index d2cdf135cc..bddef4485f 100644
--- a/elf/dl-object.c
+++ b/elf/dl-object.c
@@ -202,38 +202,29 @@  _dl_new_object (char *realname, const char *libname, int type,
 	}
       else
 	{
-	  size_t len = realname_len;
-	  char *result = NULL;
-
-	  /* Get the current directory name.  */
-	  origin = NULL;
-	  do
+	  /* The rtld __getcwd implementation does not handle paths larger
+	     than PATH_MAX (which would be invalid to be used on subsequent
+	     open calls).  */
+	  origin = __getcwd (NULL, 0);
+	  if (origin == NULL)
 	    {
-	      char *new_origin;
-
-	      len += 128;
-	      new_origin = (char *) realloc (origin, len);
-	      if (new_origin == NULL)
-		/* We exit the loop.  Note that result == NULL.  */
-		break;
-	      origin = new_origin;
+	      origin = (char *) -1;
+	      goto out;
 	    }
-	  while ((result = __getcwd (origin, len - realname_len)) == NULL
-		 && errno == ERANGE);
-
+	  size_t len = strlen (origin);
+	  char *result = realloc (origin, len + realname_len);
 	  if (result == NULL)
 	    {
-	      /* We were not able to determine the current directory.
-		 Note that free(origin) is OK if origin == NULL.  */
 	      free (origin);
 	      origin = (char *) -1;
 	      goto out;
 	    }
+	  origin = result;
+	  cp = origin + len;
 
 	  /* Find the end of the path and see whether we have to add a
 	     slash.  We could use rawmemchr but this need not be
 	     fast.  */
-	  cp = (strchr) (origin, '\0');
 	  if (cp[-1] != '/')
 	    *cp++ = '/';
 	}
diff --git a/include/unistd.h b/include/unistd.h
index f48da2c7a3..792cfdff0b 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -76,6 +76,8 @@  extern int __lchown (const char *__file, __uid_t __owner,
 		     __gid_t __group);
 extern int __chdir (const char *__path) attribute_hidden;
 extern int __fchdir (int __fd) attribute_hidden;
+extern char *__getcwd_generic (char *__buf, size_t __size) attribute_hidden;
+extern char *__getcwd_system (char *__buf, size_t __size) attribute_hidden;
 extern char *__getcwd (char *__buf, size_t __size);
 libc_hidden_proto (__getcwd)
 extern int __rmdir (const char *__path) attribute_hidden;
diff --git a/io/Makefile b/io/Makefile
index cf380f3516..57cb778790 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -46,7 +46,7 @@  routines :=								\
 	close dup dup2 dup3 pipe pipe2					\
 	creat creat64							\
 	chdir fchdir							\
-	getcwd getwd getdirname						\
+	getcwd getwd getcwd-system getdirname				\
 	chown fchown lchown fchownat					\
 	ttyname ttyname_r isatty					\
 	link linkat symlink symlinkat readlink readlinkat		\
diff --git a/sysdeps/posix/getcwd.c b/io/getcwd-generic.c
similarity index 98%
rename from sysdeps/posix/getcwd.c
rename to io/getcwd-generic.c
index 1e6fc9b845..3b4be6bdaa 100644
--- a/sysdeps/posix/getcwd.c
+++ b/io/getcwd-generic.c
@@ -89,7 +89,7 @@ 
 
 #if !_LIBC
 # define __close_nocancel_nostatus close
-# define __getcwd rpl_getcwd
+# define __getcwd_generic rpl_getcwd
 # define stat64    stat
 # define __fstat64 fstat
 # define __fstatat64 fstatat
@@ -154,7 +154,7 @@  getcwd_nothrow (char *buf, size_t size)
    bytes long, unless SIZE == 0, in which case it is as big as necessary.  */
 
 char *
-__getcwd (char *buf, size_t size)
+__getcwd_generic (char *buf, size_t size)
 {
   /* Lengths of big file name components and entire file names, and a
      deep level of file name nesting.  These numbers are not upper
@@ -486,8 +486,3 @@  __getcwd (char *buf, size_t size)
   }
   return NULL;
 }
-
-#if defined _LIBC && !defined __getcwd
-libc_hidden_def (__getcwd)
-weak_alias (__getcwd, getcwd)
-#endif
diff --git a/io/getcwd-system.c b/io/getcwd-system.c
new file mode 100644
index 0000000000..b0ae6271ab
--- /dev/null
+++ b/io/getcwd-system.c
@@ -0,0 +1,28 @@ 
+/* Architecture specific getcwd implementation.  Generic implementation.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+/* This function is called by the generic 'getcwd' implementation to allow
+   a system to implement if the it provides a faster or simpler way to obtain
+   the current direction (e.g. through a syscall).  */
+char *
+__getcwd_system (char *buf, size_t size)
+{
+  return NULL;
+}
diff --git a/io/getcwd.c b/io/getcwd.c
index 0fabd98131..0e0a790368 100644
--- a/io/getcwd.c
+++ b/io/getcwd.c
@@ -19,6 +19,11 @@ 
 #include <unistd.h>
 #include <stddef.h>
 
+/* The rtld does not need to handle path larger than PATH_MAX.  */
+#if !IS_IN(rtld)
+#include <getcwd-generic.c>
+#endif
+
 /* Get the pathname of the current working directory,
    and put it in SIZE bytes of BUF.  Returns NULL if the
    directory couldn't be determined or SIZE was too small.
@@ -29,11 +34,12 @@ 
 char *
 __getcwd (char *buf, size_t size)
 {
-  __set_errno (ENOSYS);
-  return NULL;
+  char *r = __getcwd_system (buf, size);
+#if !IS_IN(rtld)
+  if (r == NULL)
+    r = __getcwd_generic (buf, size);
+#endif
+  return r;
 }
 libc_hidden_def (__getcwd)
 weak_alias (__getcwd, getcwd)
-
-stub_warning (__getcwd)
-stub_warning (getcwd)
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 9b2a253032..465ffe7104 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -280,8 +280,7 @@  tests += tst-fallocate tst-fallocate64 tst-o_path-locks
 endif
 
 ifeq ($(subdir),elf)
-sysdep-rtld-routines += dl-brk dl-sbrk dl-getcwd dl-openat64 dl-opendir \
-			dl-fxstatat64
+sysdep-rtld-routines += dl-brk dl-sbrk dl-openat64 dl-opendir
 
 libof-lddlibc4 = lddlibc4
 
diff --git a/sysdeps/unix/sysv/linux/alpha/dl-fxstatat64.c b/sysdeps/unix/sysv/linux/alpha/dl-fxstatat64.c
deleted file mode 100644
index 330b33f7c7..0000000000
--- a/sysdeps/unix/sysv/linux/alpha/dl-fxstatat64.c
+++ /dev/null
@@ -1 +0,0 @@ 
-#include "fxstatat.c"
diff --git a/sysdeps/unix/sysv/linux/dl-fxstatat64.c b/sysdeps/unix/sysv/linux/dl-fxstatat64.c
deleted file mode 100644
index d229d0ea0f..0000000000
--- a/sysdeps/unix/sysv/linux/dl-fxstatat64.c
+++ /dev/null
@@ -1 +0,0 @@ 
-#include <fxstatat64.c>
diff --git a/sysdeps/unix/sysv/linux/dl-getcwd.c b/sysdeps/unix/sysv/linux/dl-getcwd.c
deleted file mode 100644
index 4bd5657f1e..0000000000
--- a/sysdeps/unix/sysv/linux/dl-getcwd.c
+++ /dev/null
@@ -1 +0,0 @@ 
-#include "getcwd.c"
diff --git a/sysdeps/unix/sysv/linux/getcwd-system.c b/sysdeps/unix/sysv/linux/getcwd-system.c
new file mode 100644
index 0000000000..a2df688127
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/getcwd-system.c
@@ -0,0 +1,69 @@ 
+/* Determine current working directory.  Linux version.
+   Copyright (C) 1997-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@cygnus.com>, 1997.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <sysdep.h>
+
+char *
+__getcwd_system (char *buf, size_t size)
+{
+  char *path;
+
+  size_t alloc_size = size;
+  if (size == 0)
+    {
+      if (buf != NULL)
+	{
+	  __set_errno (EINVAL);
+	  return NULL;
+	}
+
+      alloc_size = PATH_MAX;
+    }
+
+  if (buf == NULL)
+    {
+      path = malloc (alloc_size);
+      if (path == NULL)
+	return NULL;
+    }
+  else
+    path = buf;
+
+  int r = INLINE_SYSCALL_CALL (getcwd, path, alloc_size);
+  if (r > 0 && path[0] == '/')
+    {
+      if (buf == NULL && size == 0)
+	/* Ensure that the buffer is only as large as necessary.  */
+	buf = realloc (path, r);
+
+      if (buf == NULL)
+	/* Either buf was NULL all along, or `realloc' failed but
+	   we still have the original string.  */
+	buf = path;
+
+      return buf;
+    }
+
+  if (buf == NULL)
+    free (path);
+
+  return NULL;
+}
diff --git a/sysdeps/unix/sysv/linux/getcwd.c b/sysdeps/unix/sysv/linux/getcwd.c
deleted file mode 100644
index fabc4bb8cc..0000000000
--- a/sysdeps/unix/sysv/linux/getcwd.c
+++ /dev/null
@@ -1,136 +0,0 @@ 
-/* Determine current working directory.  Linux version.
-   Copyright (C) 1997-2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@cygnus.com>, 1997.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <assert.h>
-#include <errno.h>
-#include <limits.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <sys/param.h>
-
-#include <sysdep.h>
-#include <sys/syscall.h>
-
-
-/* If we compile the file for use in ld.so we don't need the feature
-   that getcwd() allocates the buffers itself.  */
-#if IS_IN (rtld)
-# define NO_ALLOCATION	1
-#endif
-
-
-/* The "proc" filesystem provides an easy method to retrieve the value.
-   For each process, the corresponding directory contains a symbolic link
-   named `cwd'.  Reading the content of this link immediate gives us the
-   information.  But we have to take care for systems which do not have
-   the proc filesystem mounted.  Use the POSIX implementation in this case.  */
-static char *generic_getcwd (char *buf, size_t size);
-
-char *
-__getcwd (char *buf, size_t size)
-{
-  char *path;
-  char *result;
-
-#ifndef NO_ALLOCATION
-  size_t alloc_size = size;
-  if (size == 0)
-    {
-      if (buf != NULL)
-	{
-	  __set_errno (EINVAL);
-	  return NULL;
-	}
-
-      alloc_size = MAX (PATH_MAX, __getpagesize ());
-    }
-
-  if (buf == NULL)
-    {
-      path = malloc (alloc_size);
-      if (path == NULL)
-	return NULL;
-    }
-  else
-#else
-# define alloc_size size
-#endif
-    path = buf;
-
-  int retval;
-
-  retval = INLINE_SYSCALL (getcwd, 2, path, alloc_size);
-  if (retval > 0 && path[0] == '/')
-    {
-#ifndef NO_ALLOCATION
-      if (buf == NULL && size == 0)
-	/* Ensure that the buffer is only as large as necessary.  */
-	buf = realloc (path, (size_t) retval);
-
-      if (buf == NULL)
-	/* Either buf was NULL all along, or `realloc' failed but
-	   we still have the original string.  */
-	buf = path;
-#endif
-
-      return buf;
-    }
-
-  /* The system call either cannot handle paths longer than a page
-     or can succeed without returning an absolute path.  Just use the
-     generic implementation right away.  */
-  if (retval >= 0 || errno == ENAMETOOLONG)
-    {
-#ifndef NO_ALLOCATION
-      if (buf == NULL && size == 0)
-	{
-	  free (path);
-	  path = NULL;
-	}
-#endif
-
-      result = generic_getcwd (path, size);
-
-#ifndef NO_ALLOCATION
-      if (result == NULL && buf == NULL && size != 0)
-	free (path);
-#endif
-
-      return result;
-    }
-
-  /* It should never happen that the `getcwd' syscall failed because
-     the buffer is too small if we allocated the buffer ourselves
-     large enough.  */
-  assert (errno != ERANGE || buf != NULL || size != 0);
-
-#ifndef NO_ALLOCATION
-  if (buf == NULL)
-    free (path);
-#endif
-
-  return NULL;
-}
-libc_hidden_def (__getcwd)
-weak_alias (__getcwd, getcwd)
-
-/* Get the code for the generic version.  */
-#define GETCWD_RETURN_TYPE	static char *
-#define __getcwd		generic_getcwd
-#include <sysdeps/posix/getcwd.c>
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/dl-fxstatat64.c b/sysdeps/unix/sysv/linux/sparc/sparc64/dl-fxstatat64.c
deleted file mode 100644
index 330b33f7c7..0000000000
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/dl-fxstatat64.c
+++ /dev/null
@@ -1 +0,0 @@ 
-#include "fxstatat.c"
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/dl-fxstatat64.c b/sysdeps/unix/sysv/linux/wordsize-64/dl-fxstatat64.c
deleted file mode 100644
index 330b33f7c7..0000000000
--- a/sysdeps/unix/sysv/linux/wordsize-64/dl-fxstatat64.c
+++ /dev/null
@@ -1 +0,0 @@ 
-#include "fxstatat.c"