diff mbox series

[1/4] Sync canonicalize with gnulib [BZ #10635]

Message ID 20200910151915.1982465-1-adhemerval.zanella@linaro.org
State New
Headers show
Series [1/4] Sync canonicalize with gnulib [BZ #10635] | expand

Commit Message

Adhemerval Zanella Sept. 10, 2020, 3:19 p.m. UTC
It sync with gnulib version 44358d4d165b with the following exceptions:

  - All the logic to which __getcwd use.

  - MAXSYMLINKS define (glibc already handles it on eloop-threshold.h).

  - The pathmax.h logic to handle //path (neither Linux nor Hurd makes
    this distinction).

  - Don't explict set ENOMEM on malloc failure (it is already handled
    by malloc itself).

  - The usage of malloca.h routines.

Checked on x86_64-linux-gnu
---
 stdlib/canonicalize.c | 244 +++++++++++++++++++++++-------------------
 1 file changed, 131 insertions(+), 113 deletions(-)

Comments

Adhemerval Zanella Oct. 27, 2020, 12:59 p.m. UTC | #1
Ping.

On 10/09/2020 12:19, Adhemerval Zanella wrote:
> It sync with gnulib version 44358d4d165b with the following exceptions:
> 
>   - All the logic to which __getcwd use.
> 
>   - MAXSYMLINKS define (glibc already handles it on eloop-threshold.h).
> 
>   - The pathmax.h logic to handle //path (neither Linux nor Hurd makes
>     this distinction).
> 
>   - Don't explict set ENOMEM on malloc failure (it is already handled
>     by malloc itself).
> 
>   - The usage of malloca.h routines.
> 
> Checked on x86_64-linux-gnu
> ---
>  stdlib/canonicalize.c | 244 +++++++++++++++++++++++-------------------
>  1 file changed, 131 insertions(+), 113 deletions(-)
> 
> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
> index cbd885a3c5..c58439b3fd 100644
> --- a/stdlib/canonicalize.c
> +++ b/stdlib/canonicalize.c
> @@ -16,8 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <assert.h>
>  #include <stdlib.h>
> +
> +#include <alloca.h>
>  #include <string.h>
>  #include <unistd.h>
>  #include <limits.h>
> @@ -29,10 +30,10 @@
>  #include <shlib-compat.h>
>  
>  /* Return the canonical absolute name of file NAME.  A canonical name
> -   does not contain any `.', `..' components nor any repeated path
> +   does not contain any ".", ".." components nor any repeated path
>     separators ('/') or symlinks.  All path components must exist.  If
>     RESOLVED is null, the result is malloc'd; otherwise, if the
> -   canonical name is PATH_MAX chars or more, returns null with `errno'
> +   canonical name is PATH_MAX chars or more, returns null with 'errno'
>     set to ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars,
>     returns the name in RESOLVED.  If the name cannot be resolved and
>     RESOLVED is non-NULL, it contains the path of the first component
> @@ -50,9 +51,9 @@ __realpath (const char *name, char *resolved)
>    if (name == NULL)
>      {
>        /* As per Single Unix Specification V2 we must return an error if
> -	 either parameter is a null pointer.  We extend this to allow
> -	 the RESOLVED parameter to be NULL in case the we are expected to
> -	 allocate the room for the return value.  */
> +         either parameter is a null pointer.  We extend this to allow
> +         the RESOLVED parameter to be NULL in case the we are expected to
> +         allocate the room for the return value.  */
>        __set_errno (EINVAL);
>        return NULL;
>      }
> @@ -60,7 +61,7 @@ __realpath (const char *name, char *resolved)
>    if (name[0] == '\0')
>      {
>        /* As per Single Unix Specification V2 we must return an error if
> -	 the name argument points to an empty string.  */
> +         the name argument points to an empty string.  */
>        __set_errno (ENOENT);
>        return NULL;
>      }
> @@ -70,152 +71,169 @@ __realpath (const char *name, char *resolved)
>  #else
>    path_max = __pathconf (name, _PC_PATH_MAX);
>    if (path_max <= 0)
> -    path_max = 1024;
> +    path_max = 8192;
>  #endif
>  
>    if (resolved == NULL)
>      {
>        rpath = malloc (path_max);
>        if (rpath == NULL)
> -	return NULL;
> +        return NULL;
>      }
>    else
>      rpath = resolved;
>    rpath_limit = rpath + path_max;
>  
> +
>    if (name[0] != '/')
>      {
>        if (!__getcwd (rpath, path_max))
> -	{
> -	  rpath[0] = '\0';
> -	  goto error;
> -	}
> -      dest = __rawmemchr (rpath, '\0');
> +        {
> +          rpath[0] = '\0';
> +          goto error;
> +        }
> +      dest = strchr (rpath, '\0');
>      }
>    else
>      {
> -      rpath[0] = '/';
> -      dest = rpath + 1;
> +      dest = rpath;
> +      *dest++ = '/';
>      }
> +  start = name;
>  
> -  for (start = end = name; *start; start = end)
> +  for (end = start; *start; start = end)
>      {
> +#ifdef _LIBC
>        struct stat64 st;
> -      int n;
> +#else
> +      struct stat st;
> +#endif
>  
>        /* Skip sequence of multiple path-separators.  */
>        while (*start == '/')
> -	++start;
> +        ++start;
>  
>        /* Find end of path component.  */
>        for (end = start; *end && *end != '/'; ++end)
> -	/* Nothing.  */;
> +        /* Nothing.  */;
>  
>        if (end - start == 0)
> -	break;
> +        break;
>        else if (end - start == 1 && start[0] == '.')
> -	/* nothing */;
> +        /* nothing */;
>        else if (end - start == 2 && start[0] == '.' && start[1] == '.')
> -	{
> -	  /* Back up to previous component, ignore if at root already.  */
> -	  if (dest > rpath + 1)
> -	    while ((--dest)[-1] != '/');
> -	}
> +        {
> +          /* Back up to previous component, ignore if at root already.  */
> +          if (dest > rpath + 1)
> +            for (--dest; dest > rpath && dest[-1] != '/'; --dest)
> +              continue;
> +        }
>        else
> -	{
> -	  size_t new_size;
> -
> -	  if (dest[-1] != '/')
> -	    *dest++ = '/';
> -
> -	  if (dest + (end - start) >= rpath_limit)
> -	    {
> -	      ptrdiff_t dest_offset = dest - rpath;
> -	      char *new_rpath;
> -
> -	      if (resolved)
> -		{
> -		  __set_errno (ENAMETOOLONG);
> -		  if (dest > rpath + 1)
> -		    dest--;
> -		  *dest = '\0';
> -		  goto error;
> -		}
> -	      new_size = rpath_limit - rpath;
> -	      if (end - start + 1 > path_max)
> -		new_size += end - start + 1;
> -	      else
> -		new_size += path_max;
> -	      new_rpath = (char *) realloc (rpath, new_size);
> -	      if (new_rpath == NULL)
> -		goto error;
> -	      rpath = new_rpath;
> -	      rpath_limit = rpath + new_size;
> -
> -	      dest = rpath + dest_offset;
> -	    }
> -
> -	  dest = __mempcpy (dest, start, end - start);
> -	  *dest = '\0';
> -
> -	  if (__lxstat64 (_STAT_VER, rpath, &st) < 0)
> -	    goto error;
> -
> -	  if (S_ISLNK (st.st_mode))
> -	    {
> -	      char *buf = __alloca (path_max);
> -	      size_t len;
> -
> -	      if (++num_links > __eloop_threshold ())
> -		{
> -		  __set_errno (ELOOP);
> -		  goto error;
> -		}
> -
> -	      n = __readlink (rpath, buf, path_max - 1);
> -	      if (n < 0)
> -		goto error;
> -	      buf[n] = '\0';
> -
> -	      if (!extra_buf)
> -		extra_buf = __alloca (path_max);
> -
> -	      len = strlen (end);
> -	      if (path_max - n <= len)
> -		{
> -		  __set_errno (ENAMETOOLONG);
> -		  goto error;
> -		}
> -
> -	      /* Careful here, end may be a pointer into extra_buf... */
> -	      memmove (&extra_buf[n], end, len + 1);
> -	      name = end = memcpy (extra_buf, buf, n);
> -
> -	      if (buf[0] == '/')
> -		dest = rpath + 1;	/* It's an absolute symlink */
> -	      else
> -		/* Back up to previous component, ignore if at root already: */
> -		if (dest > rpath + 1)
> -		  while ((--dest)[-1] != '/');
> -	    }
> -	  else if (!S_ISDIR (st.st_mode) && *end != '\0')
> -	    {
> -	      __set_errno (ENOTDIR);
> -	      goto error;
> -	    }
> -	}
> +        {
> +          size_t new_size;
> +
> +          if (dest[-1] != '/')
> +            *dest++ = '/';
> +
> +          if (dest + (end - start) >= rpath_limit)
> +            {
> +              ptrdiff_t dest_offset = dest - rpath;
> +              char *new_rpath;
> +
> +              if (resolved)
> +                {
> +                  __set_errno (ENAMETOOLONG);
> +                  if (dest > rpath + 1)
> +                    dest--;
> +                  *dest = '\0';
> +                  goto error;
> +                }
> +              new_size = rpath_limit - rpath;
> +              if (end - start + 1 > path_max)
> +                new_size += end - start + 1;
> +              else
> +                new_size += path_max;
> +              new_rpath = (char *) realloc (rpath, new_size);
> +              if (new_rpath == NULL)
> +                goto error;
> +              rpath = new_rpath;
> +              rpath_limit = rpath + new_size;
> +
> +              dest = rpath + dest_offset;
> +            }
> +
> +          dest = __mempcpy (dest, start, end - start);
> +          *dest = '\0';
> +
> +          if (__lxstat64 (_STAT_VER, rpath, &st) < 0)
> +            goto error;
> +
> +          if (S_ISLNK (st.st_mode))
> +            {
> +              char *buf = __alloca (path_max);
> +              size_t len;
> +              ssize_t n;
> +
> +              if (++num_links > __eloop_threshold ())
> +                {
> +                  __set_errno (ELOOP);
> +                  goto error;
> +                }
> +
> +              n = __readlink (rpath, buf, path_max - 1);
> +              if (n < 0)
> +                goto error;
> +              buf[n] = '\0';
> +
> +              if (!extra_buf)
> +                extra_buf = __alloca (path_max);
> +
> +              len = strlen (end);
> +              /* Check that n + len + 1 doesn't overflow and is <= path_max. */
> +              if (n >= SIZE_MAX - len || n + len >= path_max)
> +                {
> +                  __set_errno (ENAMETOOLONG);
> +                  goto error;
> +                }
> +
> +              /* Careful here, end may be a pointer into extra_buf... */
> +              memmove (&extra_buf[n], end, len + 1);
> +              name = end = memcpy (extra_buf, buf, n);
> +
> +              if (buf[0] == '/')
> +                {
> +                  dest = rpath;
> +                  *dest++ = '/'; /* It's an absolute symlink */
> +                }
> +              else
> +                {
> +                  /* Back up to previous component, ignore if at root
> +                     already: */
> +                  if (dest > rpath + 1)
> +                    for (--dest; dest > rpath && dest[-1] != '/'; --dest)
> +                      continue;
> +                }
> +            }
> +          else if (!S_ISDIR (st.st_mode) && *end != '\0')
> +            {
> +              __set_errno (ENOTDIR);
> +              goto error;
> +            }
> +        }
>      }
>    if (dest > rpath + 1 && dest[-1] == '/')
>      --dest;
>    *dest = '\0';
>  
> -  assert (resolved == NULL || resolved == rpath);
>    return rpath;
>  
>  error:
> -  assert (resolved == NULL || resolved == rpath);
> -  if (resolved == NULL)
> -    free (rpath);
> +  {
> +    int saved_errno = errno;
> +    if (resolved == NULL)
> +      free (rpath);
> +    __set_errno (saved_errno);
> +  }
>    return NULL;
>  }
>  libc_hidden_def (__realpath)
>
Andreas Schwab Oct. 27, 2020, 1:15 p.m. UTC | #2
That doesn't apply to current HEAD.  Can you rediff?

Andreas.
Adhemerval Zanella Oct. 27, 2020, 1:22 p.m. UTC | #3
On 27/10/2020 10:15, Andreas Schwab wrote:
> That doesn't apply to current HEAD.  Can you rediff?
> 
> Andreas.
> 

I will do it, thanks.
diff mbox series

Patch

diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index cbd885a3c5..c58439b3fd 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -16,8 +16,9 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
 #include <stdlib.h>
+
+#include <alloca.h>
 #include <string.h>
 #include <unistd.h>
 #include <limits.h>
@@ -29,10 +30,10 @@ 
 #include <shlib-compat.h>
 
 /* Return the canonical absolute name of file NAME.  A canonical name
-   does not contain any `.', `..' components nor any repeated path
+   does not contain any ".", ".." components nor any repeated path
    separators ('/') or symlinks.  All path components must exist.  If
    RESOLVED is null, the result is malloc'd; otherwise, if the
-   canonical name is PATH_MAX chars or more, returns null with `errno'
+   canonical name is PATH_MAX chars or more, returns null with 'errno'
    set to ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars,
    returns the name in RESOLVED.  If the name cannot be resolved and
    RESOLVED is non-NULL, it contains the path of the first component
@@ -50,9 +51,9 @@  __realpath (const char *name, char *resolved)
   if (name == NULL)
     {
       /* As per Single Unix Specification V2 we must return an error if
-	 either parameter is a null pointer.  We extend this to allow
-	 the RESOLVED parameter to be NULL in case the we are expected to
-	 allocate the room for the return value.  */
+         either parameter is a null pointer.  We extend this to allow
+         the RESOLVED parameter to be NULL in case the we are expected to
+         allocate the room for the return value.  */
       __set_errno (EINVAL);
       return NULL;
     }
@@ -60,7 +61,7 @@  __realpath (const char *name, char *resolved)
   if (name[0] == '\0')
     {
       /* As per Single Unix Specification V2 we must return an error if
-	 the name argument points to an empty string.  */
+         the name argument points to an empty string.  */
       __set_errno (ENOENT);
       return NULL;
     }
@@ -70,152 +71,169 @@  __realpath (const char *name, char *resolved)
 #else
   path_max = __pathconf (name, _PC_PATH_MAX);
   if (path_max <= 0)
-    path_max = 1024;
+    path_max = 8192;
 #endif
 
   if (resolved == NULL)
     {
       rpath = malloc (path_max);
       if (rpath == NULL)
-	return NULL;
+        return NULL;
     }
   else
     rpath = resolved;
   rpath_limit = rpath + path_max;
 
+
   if (name[0] != '/')
     {
       if (!__getcwd (rpath, path_max))
-	{
-	  rpath[0] = '\0';
-	  goto error;
-	}
-      dest = __rawmemchr (rpath, '\0');
+        {
+          rpath[0] = '\0';
+          goto error;
+        }
+      dest = strchr (rpath, '\0');
     }
   else
     {
-      rpath[0] = '/';
-      dest = rpath + 1;
+      dest = rpath;
+      *dest++ = '/';
     }
+  start = name;
 
-  for (start = end = name; *start; start = end)
+  for (end = start; *start; start = end)
     {
+#ifdef _LIBC
       struct stat64 st;
-      int n;
+#else
+      struct stat st;
+#endif
 
       /* Skip sequence of multiple path-separators.  */
       while (*start == '/')
-	++start;
+        ++start;
 
       /* Find end of path component.  */
       for (end = start; *end && *end != '/'; ++end)
-	/* Nothing.  */;
+        /* Nothing.  */;
 
       if (end - start == 0)
-	break;
+        break;
       else if (end - start == 1 && start[0] == '.')
-	/* nothing */;
+        /* nothing */;
       else if (end - start == 2 && start[0] == '.' && start[1] == '.')
-	{
-	  /* Back up to previous component, ignore if at root already.  */
-	  if (dest > rpath + 1)
-	    while ((--dest)[-1] != '/');
-	}
+        {
+          /* Back up to previous component, ignore if at root already.  */
+          if (dest > rpath + 1)
+            for (--dest; dest > rpath && dest[-1] != '/'; --dest)
+              continue;
+        }
       else
-	{
-	  size_t new_size;
-
-	  if (dest[-1] != '/')
-	    *dest++ = '/';
-
-	  if (dest + (end - start) >= rpath_limit)
-	    {
-	      ptrdiff_t dest_offset = dest - rpath;
-	      char *new_rpath;
-
-	      if (resolved)
-		{
-		  __set_errno (ENAMETOOLONG);
-		  if (dest > rpath + 1)
-		    dest--;
-		  *dest = '\0';
-		  goto error;
-		}
-	      new_size = rpath_limit - rpath;
-	      if (end - start + 1 > path_max)
-		new_size += end - start + 1;
-	      else
-		new_size += path_max;
-	      new_rpath = (char *) realloc (rpath, new_size);
-	      if (new_rpath == NULL)
-		goto error;
-	      rpath = new_rpath;
-	      rpath_limit = rpath + new_size;
-
-	      dest = rpath + dest_offset;
-	    }
-
-	  dest = __mempcpy (dest, start, end - start);
-	  *dest = '\0';
-
-	  if (__lxstat64 (_STAT_VER, rpath, &st) < 0)
-	    goto error;
-
-	  if (S_ISLNK (st.st_mode))
-	    {
-	      char *buf = __alloca (path_max);
-	      size_t len;
-
-	      if (++num_links > __eloop_threshold ())
-		{
-		  __set_errno (ELOOP);
-		  goto error;
-		}
-
-	      n = __readlink (rpath, buf, path_max - 1);
-	      if (n < 0)
-		goto error;
-	      buf[n] = '\0';
-
-	      if (!extra_buf)
-		extra_buf = __alloca (path_max);
-
-	      len = strlen (end);
-	      if (path_max - n <= len)
-		{
-		  __set_errno (ENAMETOOLONG);
-		  goto error;
-		}
-
-	      /* Careful here, end may be a pointer into extra_buf... */
-	      memmove (&extra_buf[n], end, len + 1);
-	      name = end = memcpy (extra_buf, buf, n);
-
-	      if (buf[0] == '/')
-		dest = rpath + 1;	/* It's an absolute symlink */
-	      else
-		/* Back up to previous component, ignore if at root already: */
-		if (dest > rpath + 1)
-		  while ((--dest)[-1] != '/');
-	    }
-	  else if (!S_ISDIR (st.st_mode) && *end != '\0')
-	    {
-	      __set_errno (ENOTDIR);
-	      goto error;
-	    }
-	}
+        {
+          size_t new_size;
+
+          if (dest[-1] != '/')
+            *dest++ = '/';
+
+          if (dest + (end - start) >= rpath_limit)
+            {
+              ptrdiff_t dest_offset = dest - rpath;
+              char *new_rpath;
+
+              if (resolved)
+                {
+                  __set_errno (ENAMETOOLONG);
+                  if (dest > rpath + 1)
+                    dest--;
+                  *dest = '\0';
+                  goto error;
+                }
+              new_size = rpath_limit - rpath;
+              if (end - start + 1 > path_max)
+                new_size += end - start + 1;
+              else
+                new_size += path_max;
+              new_rpath = (char *) realloc (rpath, new_size);
+              if (new_rpath == NULL)
+                goto error;
+              rpath = new_rpath;
+              rpath_limit = rpath + new_size;
+
+              dest = rpath + dest_offset;
+            }
+
+          dest = __mempcpy (dest, start, end - start);
+          *dest = '\0';
+
+          if (__lxstat64 (_STAT_VER, rpath, &st) < 0)
+            goto error;
+
+          if (S_ISLNK (st.st_mode))
+            {
+              char *buf = __alloca (path_max);
+              size_t len;
+              ssize_t n;
+
+              if (++num_links > __eloop_threshold ())
+                {
+                  __set_errno (ELOOP);
+                  goto error;
+                }
+
+              n = __readlink (rpath, buf, path_max - 1);
+              if (n < 0)
+                goto error;
+              buf[n] = '\0';
+
+              if (!extra_buf)
+                extra_buf = __alloca (path_max);
+
+              len = strlen (end);
+              /* Check that n + len + 1 doesn't overflow and is <= path_max. */
+              if (n >= SIZE_MAX - len || n + len >= path_max)
+                {
+                  __set_errno (ENAMETOOLONG);
+                  goto error;
+                }
+
+              /* Careful here, end may be a pointer into extra_buf... */
+              memmove (&extra_buf[n], end, len + 1);
+              name = end = memcpy (extra_buf, buf, n);
+
+              if (buf[0] == '/')
+                {
+                  dest = rpath;
+                  *dest++ = '/'; /* It's an absolute symlink */
+                }
+              else
+                {
+                  /* Back up to previous component, ignore if at root
+                     already: */
+                  if (dest > rpath + 1)
+                    for (--dest; dest > rpath && dest[-1] != '/'; --dest)
+                      continue;
+                }
+            }
+          else if (!S_ISDIR (st.st_mode) && *end != '\0')
+            {
+              __set_errno (ENOTDIR);
+              goto error;
+            }
+        }
     }
   if (dest > rpath + 1 && dest[-1] == '/')
     --dest;
   *dest = '\0';
 
-  assert (resolved == NULL || resolved == rpath);
   return rpath;
 
 error:
-  assert (resolved == NULL || resolved == rpath);
-  if (resolved == NULL)
-    free (rpath);
+  {
+    int saved_errno = errno;
+    if (resolved == NULL)
+      free (rpath);
+    __set_errno (saved_errno);
+  }
   return NULL;
 }
 libc_hidden_def (__realpath)