| Message ID | 20200910151915.1982465-4-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] Sync canonicalize with gnulib [BZ #10635] | expand |
Ping. On 10/09/2020 12:19, Adhemerval Zanella wrote: > The readlink already tells whether the file is a symlink, so there is > no need to call lstat to check it. However for '..' it requires an > extra readlink check if the previous component can be really accessed, > otherwise the next iteration will check a possible valid path and end > early. It should performance-wise acceptable and a gain over lstat, > afaik symlink should not update any inode information. > > Checked on x86_64-linux-gnu and i686-linux-gnu. > --- > stdlib/canonicalize.c | 52 ++++++++++++++++++++++--------------------- > 1 file changed, 27 insertions(+), 25 deletions(-) > > diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c > index 44a25a9a59..952f4dca41 100644 > --- a/stdlib/canonicalize.c > +++ b/stdlib/canonicalize.c > @@ -55,6 +55,7 @@ __realpath (const char *name, char *resolved) > const char *start, *end, *rpath_limit; > const size_t path_max = PATH_MAX; > int num_links = 0; > + char buf[PATH_MAX]; > char extra_buf[PATH_MAX]; > > if (name == NULL) > @@ -104,12 +105,6 @@ __realpath (const char *name, char *resolved) > > for (end = start; *start; start = end) > { > -#ifdef _LIBC > - struct stat64 st; > -#else > - struct stat st; > -#endif > - > /* Skip sequence of multiple path-separators. */ > while (*start == '/') > ++start; > @@ -118,12 +113,25 @@ __realpath (const char *name, char *resolved) > for (end = start; *end && *end != '/'; ++end) > /* Nothing. */; > > - if (end - start == 0) > - break; > - else if (end - start == 1 && start[0] == '.') > + if (end - start == 1 && start[0] == '.') > /* nothing */; > else if (end - start == 2 && start[0] == '.' && start[1] == '.') > { > + ssize_t n; > + > + if (dest[-1] != '/') > + *dest++ = '/'; > + *dest = '\0'; > + > + n = __readlink (rpath, buf, path_max - 1); > + if (n == -1) > + { > + if (errno == ENOTDIR && dest[-1] == '/') > + dest[-1] = '\0'; > + if (errno != EINVAL) > + goto error; > + } > + > /* Back up to previous component, ignore if at root already. */ > if (dest > rpath + 1) > for (--dest; dest > rpath && dest[-1] != '/'; --dest) > @@ -132,6 +140,7 @@ __realpath (const char *name, char *resolved) > else > { > size_t new_size; > + ssize_t n; > > if (dest[-1] != '/') > *dest++ = '/'; > @@ -166,25 +175,23 @@ __realpath (const char *name, char *resolved) > dest = __mempcpy (dest, start, end - start); > *dest = '\0'; > > - if (__lxstat64 (_STAT_VER, rpath, &st) < 0) > - goto error; > - > - if (S_ISLNK (st.st_mode)) > + n = __readlink (rpath, buf, path_max - 1); > + if (n < 0) > + { > + if (errno == ENOTDIR && dest[-1] == '/') > + dest[-1] = '\0'; > + if (errno != EINVAL) > + goto error; > + } > + else > { > - char buf[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'; > - > len = strlen (end); > /* Check that n + len + 1 doesn't overflow and is <= path_max. */ > if (n >= SIZE_MAX - len || n + len >= path_max) > @@ -211,11 +218,6 @@ __realpath (const char *name, char *resolved) > continue; > } > } > - else if (!S_ISDIR (st.st_mode) && *end != '\0') > - { > - __set_errno (ENOTDIR); > - goto error; > - } > } > } > if (dest > rpath + 1 && dest[-1] == '/') >
diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 44a25a9a59..952f4dca41 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -55,6 +55,7 @@ __realpath (const char *name, char *resolved) const char *start, *end, *rpath_limit; const size_t path_max = PATH_MAX; int num_links = 0; + char buf[PATH_MAX]; char extra_buf[PATH_MAX]; if (name == NULL) @@ -104,12 +105,6 @@ __realpath (const char *name, char *resolved) for (end = start; *start; start = end) { -#ifdef _LIBC - struct stat64 st; -#else - struct stat st; -#endif - /* Skip sequence of multiple path-separators. */ while (*start == '/') ++start; @@ -118,12 +113,25 @@ __realpath (const char *name, char *resolved) for (end = start; *end && *end != '/'; ++end) /* Nothing. */; - if (end - start == 0) - break; - else if (end - start == 1 && start[0] == '.') + if (end - start == 1 && start[0] == '.') /* nothing */; else if (end - start == 2 && start[0] == '.' && start[1] == '.') { + ssize_t n; + + if (dest[-1] != '/') + *dest++ = '/'; + *dest = '\0'; + + n = __readlink (rpath, buf, path_max - 1); + if (n == -1) + { + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno != EINVAL) + goto error; + } + /* Back up to previous component, ignore if at root already. */ if (dest > rpath + 1) for (--dest; dest > rpath && dest[-1] != '/'; --dest) @@ -132,6 +140,7 @@ __realpath (const char *name, char *resolved) else { size_t new_size; + ssize_t n; if (dest[-1] != '/') *dest++ = '/'; @@ -166,25 +175,23 @@ __realpath (const char *name, char *resolved) dest = __mempcpy (dest, start, end - start); *dest = '\0'; - if (__lxstat64 (_STAT_VER, rpath, &st) < 0) - goto error; - - if (S_ISLNK (st.st_mode)) + n = __readlink (rpath, buf, path_max - 1); + if (n < 0) + { + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno != EINVAL) + goto error; + } + else { - char buf[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'; - len = strlen (end); /* Check that n + len + 1 doesn't overflow and is <= path_max. */ if (n >= SIZE_MAX - len || n + len >= path_max) @@ -211,11 +218,6 @@ __realpath (const char *name, char *resolved) continue; } } - else if (!S_ISDIR (st.st_mode) && *end != '\0') - { - __set_errno (ENOTDIR); - goto error; - } } } if (dest > rpath + 1 && dest[-1] == '/')