* [PATCH 0/5] Fix multiple realpath issues
@ 2020-12-24 15:16 Adhemerval Zanella
2020-12-24 15:16 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Adhemerval Zanella @ 2020-12-24 15:16 UTC (permalink / raw)
To: libc-alpha, Paul Eggert; +Cc: bug-gnulib
This is an updated version of my previous set to fix multiple realpath
implementation issues [1].
For this version I used the most recent gnulib version to sync, since
it constains multiple issues and it fixes glibc BZ #10635, BZ #26592,
and BZ #26241. However, the gnulib version shows some regressions
on stdlib/tst-canon.c which I fixed on last patch (which also fixes
glibc BZ #24970).
This idea is to either apply this whole set or work with gnulib to
fix the glibc regression on their side and sync a fully working
implementation back to glibc.
[1] https://patchwork.sourceware.org/project/glibc/list/?series=1062
*** BLURB HERE ***
Adhemerval Zanella (5):
stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ
#26241]
Import idx.h from gnulib
Import filename.h from gnulib
stdlib: Add testcase fro BZ #26241
stdlib: Remove lstat usage from realpath [BZ #24970]
include/filename.h | 110 +++++
include/idx.h | 113 +++++
include/scratch_buffer.h | 21 +
stdlib/Makefile | 3 +-
stdlib/canonicalize.c | 467 ++++++++++++------
stdlib/tst-canon-bz26341.c | 108 ++++
support/support_set_small_thread_stack_size.c | 12 +-
support/xthread.h | 2 +
8 files changed, 676 insertions(+), 160 deletions(-)
create mode 100644 include/filename.h
create mode 100644 include/idx.h
create mode 100644 stdlib/tst-canon-bz26341.c
--
2.25.1
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella @ 2020-12-24 15:16 ` Adhemerval Zanella 2020-12-24 22:45 ` Paul Eggert 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella ` (3 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-24 15:16 UTC (permalink / raw) To: libc-alpha, Paul Eggert; +Cc: bug-gnulib It sync with gnulib version d9c121346 with the following difference require fix a glibc build: --- ../../gnulib/gnulib-lib/lib/canonicalize-lgpl.c +++ stdlib/canonicalize.c @@ -46,7 +46,7 @@ # define FILE_SYSTEM_PREFIX_LEN(name) 0 # define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name)) # define ISSLASH(c) ((c) == '/') -# define freea(p) ((void) (p)) +# define FUNC_REALPATH_WORKS 1 #else # define __canonicalize_file_name canonicalize_file_name # define __realpath realpath @@ -270,7 +270,7 @@ buf[n] = '\0'; char *extra_buf = extra_buffer.data; - idx_t end_idx; + idx_t end_idx = 0; if (end_in_extra_buffer) end_idx = end - extra_buf; idx_t len = strlen (end); It shows the following testcase failures: FAIL: stdlib/test-canon azanella@birita:~/Projects/glibc/build/x86_64-linux-gnu$ cat stdlib/test-canon.out stdlib/test-canon: flunked test 30 (expected resolved `./doesExist/someFile', got `[builddir]/stdlib/doesExist/someFile/') stdlib/test-canon: flunked test 31 (expected `NULL', got `[builddir]/stdlib/doesExist') 2 errors. (the [builddir] the build directory I used) This regression will be fixed in the last commit of this set. The sync also fixes BZ#26592 and BZ#26241. --- stdlib/canonicalize.c | 456 +++++++++++++++++++++++++++--------------- 1 file changed, 297 insertions(+), 159 deletions(-) diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 3fcb399a5d..66685f4526 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -16,43 +16,126 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#include <assert.h> +#ifndef _LIBC +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + optimizes away the name == NULL test below. */ +# define _GL_ARG_NONNULL(params) + +# define _GL_USE_STDLIB_ALLOC 1 +# include <libc-config.h> +#endif + +/* Specification. */ #include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include <limits.h> -#include <sys/stat.h> + #include <errno.h> +#include <limits.h> +#include <stdbool.h> #include <stddef.h> +#include <string.h> +#include <sys/stat.h> +#include <unistd.h> + +#include <scratch_buffer.h> + +#ifdef _LIBC +# include <eloop-threshold.h> +# include <shlib-compat.h> +typedef ptrdiff_t idx_t; +# define IDX_MAX PTRDIFF_MAX +# define FILE_SYSTEM_PREFIX_LEN(name) 0 +# define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name)) +# define ISSLASH(c) ((c) == '/') +# define FUNC_REALPATH_WORKS 1 +#else +# define __canonicalize_file_name canonicalize_file_name +# define __realpath realpath +# include "idx.h" +# include "pathmax.h" +# include "filename.h" +# if defined _WIN32 && !defined __CYGWIN__ +# define __getcwd _getcwd +# elif HAVE_GETCWD +# if IN_RELOCWRAPPER + /* When building the relocatable program wrapper, use the system's getcwd + function, not the gnulib override, otherwise we would get a link error. + */ +# undef getcwd +# endif +# if defined VMS && !defined getcwd + /* We want the directory in Unix syntax, not in VMS syntax. + The gnulib override of 'getcwd' takes 2 arguments; the original VMS + 'getcwd' takes 3 arguments. */ +# define __getcwd(buf, max) getcwd (buf, max, 0) +# else +# define __getcwd getcwd +# endif +# else +# define __getcwd(buf, max) getwd (buf) +# endif +# define __mempcpy mempcpy +# define __pathconf pathconf +# define __rawmemchr rawmemchr +# define __readlink readlink +# ifndef MAXSYMLINKS +# ifdef SYMLOOP_MAX +# define MAXSYMLINKS SYMLOOP_MAX +# else +# define MAXSYMLINKS 20 +# endif +# endif +# define __eloop_threshold() MAXSYMLINKS +#endif + +#ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT +# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0 +#endif + +#if !FUNC_REALPATH_WORKS || defined _LIBC -#include <eloop-threshold.h> -#include <shlib-compat.h> +static idx_t +get_path_max (void) +{ +# ifdef PATH_MAX + long int path_max = PATH_MAX; +# else + /* The caller invoked realpath with a null RESOLVED, even though + PATH_MAX is not defined as a constant. The glibc manual says + programs should not do this, and POSIX says the behavior is undefined. + Historically, glibc here used the result of pathconf, or 1024 if that + failed; stay consistent with this (dubious) historical practice. */ + int err = errno; + long int path_max = __pathconf ("/", _PC_PATH_MAX); + __set_errno (err); +# endif + return path_max < 0 ? 1024 : path_max <= IDX_MAX ? path_max : IDX_MAX; +} /* Return the canonical absolute name of file NAME. A canonical name - does not contain any `.', `..' components nor any repeated path - separators ('/') or symlinks. All path components must exist. If + does not contain any ".", ".." components nor any repeated file name + separators ('/') or symlinks. All file name 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 - that cannot be resolved. If the path can be resolved, RESOLVED + RESOLVED is non-NULL, it contains the name of the first component + that cannot be resolved. If the name can be resolved, RESOLVED holds the same value as the value returned. */ char * __realpath (const char *name, char *resolved) { - char *rpath, *dest, *extra_buf = NULL; - const char *start, *end, *rpath_limit; - long int path_max; + char *dest; + char const *start; + char const *end; int num_links = 0; 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,166 +143,221 @@ __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; } -#ifdef PATH_MAX - path_max = PATH_MAX; -#else - path_max = __pathconf (name, _PC_PATH_MAX); - if (path_max <= 0) - path_max = 1024; -#endif - - if (resolved == NULL) - { - rpath = malloc (path_max); - if (rpath == NULL) - return NULL; - } - else - rpath = resolved; - rpath_limit = rpath + path_max; - - if (name[0] != '/') + struct scratch_buffer extra_buffer, link_buffer; + struct scratch_buffer rname_buffer; + struct scratch_buffer *rname_buf = &rname_buffer; + scratch_buffer_init (&extra_buffer); + scratch_buffer_init (&link_buffer); + scratch_buffer_init (rname_buf); + char *rname_on_stack = rname_buf->data; + char *rname = rname_on_stack; + bool end_in_extra_buffer = false; + bool failed = true; + + /* This is always zero for Posix hosts, but can be 2 for MS-Windows + and MS-DOS X:/foo/bar file names. */ + idx_t prefix_len = FILE_SYSTEM_PREFIX_LEN (name); + + if (!IS_ABSOLUTE_FILE_NAME (name)) { - if (!__getcwd (rpath, path_max)) - { - rpath[0] = '\0'; - goto error; - } - dest = __rawmemchr (rpath, '\0'); + while (!__getcwd (rname, rname_buf->length)) + { + if (errno != ERANGE) + { + dest = rname; + goto error; + } + if (!scratch_buffer_grow (rname_buf)) + goto error_nomem; + rname = rname_buf->data; + } + dest = __rawmemchr (rname, '\0'); + start = name; + prefix_len = FILE_SYSTEM_PREFIX_LEN (rname); } else { - rpath[0] = '/'; - dest = rpath + 1; + dest = __mempcpy (rname, name, prefix_len); + *dest++ = '/'; + if (DOUBLE_SLASH_IS_DISTINCT_ROOT) + { + if (prefix_len == 0 /* implies ISSLASH (name[0]) */ + && ISSLASH (name[1]) && !ISSLASH (name[2])) + *dest++ = '/'; + *dest = '\0'; + } + start = name + prefix_len; } - for (start = end = name; *start; start = end) + for ( ; *start; start = end) { - struct stat64 st; - int n; - - /* Skip sequence of multiple path-separators. */ - while (*start == '/') - ++start; - - /* Find end of path component. */ - for (end = start; *end && *end != '/'; ++end) - /* Nothing. */; - - if (end - start == 0) - break; - else if (end - start == 1 && start[0] == '.') - /* 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] != '/'); - } + /* Skip sequence of multiple file name separators. */ + while (ISSLASH (*start)) + ++start; + + /* Find end of component. */ + for (end = start; *end && !ISSLASH (*end); ++end) + /* Nothing. */; + + /* Length of this file name component; it can be zero if a file + name ends in '/'. */ + idx_t startlen = end - start; + + if (startlen == 1 && start[0] == '.') + /* nothing */; + else if (startlen == 2 && start[0] == '.' && start[1] == '.') + { + /* Back up to previous component, ignore if at root already. */ + if (dest > rname + prefix_len + 1) + for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest) + continue; + if (DOUBLE_SLASH_IS_DISTINCT_ROOT + && dest == rname + 1 && !prefix_len + && ISSLASH (*dest) && !ISSLASH (dest[1])) + dest++; + } 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 (__lstat64 (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; - } - } + { + if (!ISSLASH (dest[-1])) + *dest++ = '/'; + + while (rname + rname_buf->length - dest <= startlen) + { + idx_t dest_offset = dest - rname; + if (!scratch_buffer_grow_preserve (rname_buf)) + goto error_nomem; + rname = rname_buf->data; + dest = rname + dest_offset; + } + + dest = __mempcpy (dest, start, startlen); + *dest = '\0'; + + /* If STARTLEN == 0, RNAME ends in '/'; use stat rather than + readlink, because readlink might fail with EINVAL without + checking whether RNAME sans '/' is valid. */ + struct stat st; + char *buf = NULL; + ssize_t n; + if (startlen != 0) + { + while (true) + { + buf = link_buffer.data; + idx_t bufsize = link_buffer.length; + n = __readlink (rname, buf, bufsize - 1); + if (n < bufsize - 1) + break; + if (!scratch_buffer_grow (&link_buffer)) + goto error_nomem; + } + if (n < 0) + buf = NULL; + } + if (buf) + { + if (++num_links > __eloop_threshold ()) + { + __set_errno (ELOOP); + goto error; + } + + buf[n] = '\0'; + + char *extra_buf = extra_buffer.data; + idx_t end_idx = 0; + if (end_in_extra_buffer) + end_idx = end - extra_buf; + idx_t len = strlen (end); + while (extra_buffer.length <= len + n) + { + if (!scratch_buffer_grow_preserve (&extra_buffer)) + goto error_nomem; + extra_buf = extra_buffer.data; + } + if (end_in_extra_buffer) + end = extra_buf + end_idx; + + /* Careful here, end may be a pointer into extra_buf... */ + memmove (&extra_buf[n], end, len + 1); + name = end = memcpy (extra_buf, buf, n); + end_in_extra_buffer = true; + + if (IS_ABSOLUTE_FILE_NAME (buf)) + { + idx_t pfxlen = FILE_SYSTEM_PREFIX_LEN (buf); + + dest = __mempcpy (rname, buf, pfxlen); + *dest++ = '/'; /* It's an absolute symlink */ + if (DOUBLE_SLASH_IS_DISTINCT_ROOT) + { + if (ISSLASH (buf[1]) && !ISSLASH (buf[2]) && !pfxlen) + *dest++ = '/'; + *dest = '\0'; + } + /* Install the new prefix to be in effect hereafter. */ + prefix_len = pfxlen; + } + else + { + /* Back up to previous component, ignore if at root + already: */ + if (dest > rname + prefix_len + 1) + for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest) + continue; + if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 + && ISSLASH (*dest) && !ISSLASH (dest[1]) && !prefix_len) + dest++; + } + } + else if (! (startlen == 0 + ? stat (rname, &st) == 0 || errno == EOVERFLOW + : errno == EINVAL)) + goto error; + } } - if (dest > rpath + 1 && dest[-1] == '/') + if (dest > rname + prefix_len + 1 && ISSLASH (dest[-1])) --dest; - *dest = '\0'; - - assert (resolved == NULL || resolved == rpath); - return rpath; + if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len + && ISSLASH (*dest) && !ISSLASH (dest[1])) + dest++; + failed = false; error: - assert (resolved == NULL || resolved == rpath); - if (resolved == NULL) - free (rpath); - return NULL; + *dest++ = '\0'; + if (resolved != NULL && dest - rname <= get_path_max ()) + rname = strcpy (resolved, rname); + +error_nomem: + scratch_buffer_free (&extra_buffer); + scratch_buffer_free (&link_buffer); + if (failed || rname == resolved) + scratch_buffer_free (rname_buf); + + if (failed) + return NULL; + + if (rname == resolved) + return rname; + idx_t rname_size = dest - rname; + if (rname == rname_on_stack) + { + rname = malloc (rname_size); + if (rname == NULL) + return NULL; + return memcpy (rname, rname_on_stack, rname_size); + } + char *result = realloc (rname, rname_size); + return result != NULL ? result : rname; } libc_hidden_def (__realpath) versioned_symbol (libc, __realpath, realpath, GLIBC_2_3); +#endif /* !FUNC_REALPATH_WORKS || defined _LIBC */ #if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_3) -- 2.25.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] 2020-12-24 15:16 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella @ 2020-12-24 22:45 ` Paul Eggert 2020-12-25 0:44 ` [PATCH 1/5] warnings in canonicalize.c Bruno Haible 2020-12-28 12:53 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella 0 siblings, 2 replies; 22+ messages in thread From: Paul Eggert @ 2020-12-24 22:45 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: bug-gnulib, libc-alpha On 12/24/20 7:16 AM, Adhemerval Zanella wrote: > It sync with gnulib version d9c121346 with the following difference > require fix a glibc build: As I mentioned today on bug-gnulib I installed some changes into Gnulib which are related, and it'd probably be better to sync with the latest version. (Unfortunately I didn't know about your changes, so there was some overlapping work here.) > +# define FUNC_REALPATH_WORKS 1 I don't see why this change is needed, as the only use of that macro is in "#if !FUNC_REALPATH_WORKS || defined _LIBC", which is unaffected by this change since _LIBC is defined. > @@ -270,7 +270,7 @@ > buf[n] = '\0'; > > char *extra_buf = extra_buffer.data; > - idx_t end_idx; > + idx_t end_idx = 0; > if (end_in_extra_buffer) > end_idx = end - extra_buf; > idx_t len = strlen (end); This change isn't needed, since end_idx is used only when end_in_extra_buffer is true. Was the latter change put in only to pacify older GCC versions? (I don't get a warning with GCC 10.2.) If so, we should do the initialization only for those older versions; or Gnulib has a GCC_LINT feature for this sort of thing. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] warnings in canonicalize.c 2020-12-24 22:45 ` Paul Eggert @ 2020-12-25 0:44 ` Bruno Haible 2020-12-25 5:56 ` Paul Eggert 2020-12-28 12:53 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella 1 sibling, 1 reply; 22+ messages in thread From: Bruno Haible @ 2020-12-25 0:44 UTC (permalink / raw) To: bug-gnulib; +Cc: Paul Eggert, Adhemerval Zanella, libc-alpha Paul Eggert wrote: > > @@ -270,7 +270,7 @@ > > buf[n] = '\0'; > > > > char *extra_buf = extra_buffer.data; > > - idx_t end_idx; > > + idx_t end_idx = 0; > > if (end_in_extra_buffer) > > end_idx = end - extra_buf; > > idx_t len = strlen (end); > > This change isn't needed, since end_idx is used only when > end_in_extra_buffer is true. > > Was the latter change put in only to pacify older GCC versions? Yes. > (I don't get a warning with GCC 10.2.) Indeed, I don't see warnings in canonicalize-lgpl.c. But there are warnings in gnulib's canonicalize.c: gcc 9.3.0: $ gcc -DHAVE_CONFIG_H -I. -I.. -Wall -g -O2 -c ./canonicalize.c ./canonicalize.c: In function 'canonicalize_filename_mode_stk': ./canonicalize.c:337:16: warning: unused variable 'discard' [-Wunused-variable] 337 | char discard; | ^~~~~~~ ./canonicalize.c: In function 'canonicalize_filename_mode': ./canonicalize.c:400:33: warning: 'end_idx' may be used uninitialized in this function [-Wmaybe-uninitialized] 400 | end = extra_buf + end_idx; | ~~~~~~~~~~^~~~~~~~~ ./canonicalize.c:389:21: note: 'end_idx' was declared here 389 | idx_t end_idx; | ^~~~~~~ gcc 10.2.0: $ gcc -DHAVE_CONFIG_H -I. -I.. -Wall -g -O2 -c ./canonicalize.c ./canonicalize.c: In function 'canonicalize_filename_mode_stk': ./canonicalize.c:337:16: warning: unused variable 'discard' [-Wunused-variable] 337 | char discard; | ^~~~~~~ ... ./canonicalize.c:400:33: warning: 'end_idx' may be used uninitialized in this function [-Wmaybe-uninitialized] 400 | end = extra_buf + end_idx; | ~~~~~~~~~~^~~~~~~~~ ./canonicalize.c:389:21: note: 'end_idx' was declared here 389 | idx_t end_idx; | ^~~~~~~ Bruno ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] warnings in canonicalize.c 2020-12-25 0:44 ` [PATCH 1/5] warnings in canonicalize.c Bruno Haible @ 2020-12-25 5:56 ` Paul Eggert 0 siblings, 0 replies; 22+ messages in thread From: Paul Eggert @ 2020-12-25 5:56 UTC (permalink / raw) To: Bruno Haible; +Cc: Adhemerval Zanella, libc-alpha, bug-gnulib [-- Attachment #1: Type: text/plain, Size: 696 bytes --] On 12/24/20 4:44 PM, Bruno Haible wrote: > Indeed, I don't see warnings in canonicalize-lgpl.c. But there are warnings > in gnulib's canonicalize.c: Thanks, I fixed those, along one or two others I noticed when compiling with GCC (Ubuntu 10.2.0-13ubuntu1). Even though I didn't get these warnings with canonicalize-lgpl.c I guess they're likely to occur and anyway it's better to keep the two files in sync when possible. So I installed the attached patch, which should fix the issues you noticed and should also propagate the warnings patches into canonicalize-lgpl.c. I hope this fixes the bogus warnings that Adhemerval noticed when building glibc's stdlib/canonicalize.c. [-- Attachment #2: 0001-canonicalize-canonicalize-lgpl-remove-lint.patch --] [-- Type: text/x-patch, Size: 5236 bytes --] From abc2b3fe5af97e971f10ebdbeee174f6d87f1fe6 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 24 Dec 2020 21:33:18 -0800 Subject: [PATCH] canonicalize, canonicalize-lgpl: remove lint Pacify GCC. Some of these problems were reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2020-12/msg00217.html * lib/canonicalize-lgpl.c, lib/canonicalize.c: Sort shared include directives, for consistency. (IF_LINT): New macro. (suffix_requires_dir_check): Mark with _GL_ATTRIBUTE_PURE. * lib/canonicalize-lgpl.c (GCC_LINT, _GL_ATTRIBUTE_PURE) [_LIBC]: New macros. (realpath_stk): Suppress bogus -Wmaybe-uninitialized warning. * lib/canonicalize.c (canonicalize_filename_mode_stk): Omit unused local. Suppress bogus -Wmaybe-uninitialized warning. --- ChangeLog | 15 +++++++++++++++ lib/canonicalize-lgpl.c | 15 ++++++++++++--- lib/canonicalize.c | 16 +++++++++++----- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8f27b286e..dff97afe4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2020-12-24 Paul Eggert <eggert@cs.ucla.edu> + + canonicalize, canonicalize-lgpl: remove lint + Pacify GCC. Some of these problems were reported by Bruno Haible in: + https://lists.gnu.org/r/bug-gnulib/2020-12/msg00217.html + * lib/canonicalize-lgpl.c, lib/canonicalize.c: + Sort shared include directives, for consistency. + (IF_LINT): New macro. + (suffix_requires_dir_check): Mark with _GL_ATTRIBUTE_PURE. + * lib/canonicalize-lgpl.c (GCC_LINT, _GL_ATTRIBUTE_PURE) [_LIBC]: + New macros. + (realpath_stk): Suppress bogus -Wmaybe-uninitialized warning. + * lib/canonicalize.c (canonicalize_filename_mode_stk): + Omit unused local. Suppress bogus -Wmaybe-uninitialized warning. + 2020-12-24 Bruno Haible <bruno@clisp.org> spawn-pipe: Use posix_spawn by default on native Windows. diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 8c8f98abc..68b6fdf6e 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -38,8 +38,8 @@ #include <unistd.h> #include <eloop-threshold.h> -#include <idx.h> #include <filename.h> +#include <idx.h> #include <scratch_buffer.h> #ifdef _LIBC @@ -50,6 +50,8 @@ # else # define FACCESSAT_NEVER_EOVERFLOWS true # endif +# define GCC_LINT 1 +# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__)) #else # define __canonicalize_file_name canonicalize_file_name # define __realpath realpath @@ -82,6 +84,13 @@ # define __stat stat #endif +/* Suppress bogus GCC -Wmaybe-uninitialized warnings. */ +#if defined GCC_LINT || defined lint +# define IF_LINT(Code) Code +#else +# define IF_LINT(Code) /* empty */ +#endif + #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false #endif @@ -114,7 +123,7 @@ file_accessible (char const *file) component within END. END must either be empty, or start with a slash. */ -static bool +static bool _GL_ATTRIBUTE_PURE suffix_requires_dir_check (char const *end) { /* If END does not start with a slash, the suffix is OK. */ @@ -338,7 +347,7 @@ realpath_stk (const char *name, char *resolved, buf[n] = '\0'; char *extra_buf = extra_buffer.data; - idx_t end_idx; + idx_t end_idx IF_LINT (= 0); if (end_in_extra_buffer) end_idx = end - extra_buf; idx_t len = strlen (end); diff --git a/lib/canonicalize.c b/lib/canonicalize.c index 2c88335bf..e8b74d855 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -27,14 +27,21 @@ #include <sys/stat.h> #include <unistd.h> +#include <filename.h> +#include <idx.h> #include <scratch_buffer.h> #include "attribute.h" #include "file-set.h" -#include "idx.h" #include "hash-triple.h" #include "xalloc.h" -#include "filename.h" + +/* Suppress bogus GCC -Wmaybe-uninitialized warnings. */ +#if defined GCC_LINT || defined lint +# define IF_LINT(Code) Code +#else +# define IF_LINT(Code) /* empty */ +#endif #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false @@ -72,7 +79,7 @@ file_accessible (char const *file) component within END. END must either be empty, or start with a slash. */ -static bool +static bool _GL_ATTRIBUTE_PURE suffix_requires_dir_check (char const *end) { /* If END does not start with a slash, the suffix is OK. */ @@ -334,7 +341,6 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode, dest = mempcpy (dest, start, startlen); *dest = '\0'; - char discard; char *buf; ssize_t n = -1; if (!logical) @@ -386,7 +392,7 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode, buf[n] = '\0'; char *extra_buf = extra_buffer.data; - idx_t end_idx; + idx_t end_idx IF_LINT (= 0); if (end_in_extra_buffer) end_idx = end - extra_buf; idx_t len = strlen (end); -- 2.27.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] 2020-12-24 22:45 ` Paul Eggert 2020-12-25 0:44 ` [PATCH 1/5] warnings in canonicalize.c Bruno Haible @ 2020-12-28 12:53 ` Adhemerval Zanella 1 sibling, 0 replies; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-28 12:53 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib, libc-alpha On 24/12/2020 19:45, Paul Eggert wrote: > On 12/24/20 7:16 AM, Adhemerval Zanella wrote: >> It sync with gnulib version d9c121346 with the following difference >> require fix a glibc build: > > As I mentioned today on bug-gnulib I installed some changes into Gnulib which are related, and it'd probably be better to sync with the latest version. (Unfortunately I didn't know about your changes, so there was some overlapping work here.) > >> +# define FUNC_REALPATH_WORKS 1 > > I don't see why this change is needed, as the only use of that macro is in "#if !FUNC_REALPATH_WORKS || defined _LIBC", which is unaffected by this change since _LIBC is defined. I am seeing this with gcc 9.2.1: canonicalize.c:101:6: error: "FUNC_REALPATH_WORKS" is not defined, evaluates to 0 [-Werror=undef] 101 | #if !FUNC_REALPATH_WORKS || defined _LIBC (We build glibc with -Wundef -Werror). ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] Import idx.h from gnulib 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella 2020-12-24 15:16 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella @ 2020-12-24 15:16 ` Adhemerval Zanella 2020-12-24 22:53 ` Paul Eggert ` (2 more replies) 2020-12-24 15:16 ` [PATCH 3/5] Import filename.h " Adhemerval Zanella ` (2 subsequent siblings) 4 siblings, 3 replies; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-24 15:16 UTC (permalink / raw) To: libc-alpha, Paul Eggert; +Cc: bug-gnulib [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=ojwedtwnpqxshmrr, Size: 5617 bytes --] And use to simplify stdlib/canonicalize.c implementation. --- include/idx.h | 113 ++++++++++++++++++++++++++++++++++++++++++ stdlib/canonicalize.c | 5 +- 2 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 include/idx.h diff --git a/include/idx.h b/include/idx.h new file mode 100644 index 0000000000..ad7d31a2bc --- /dev/null +++ b/include/idx.h @@ -0,0 +1,113 @@ +/* A type for indices and sizes. + + Copyright (C) 2020 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2, or (at your option) + any later version. + + This program 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see <https://www.gnu.org/licenses/>. */ + +#ifndef _IDX_H +#define _IDX_H + +/* Get ptrdiff_t. */ +#include <stddef.h> + +/* Get PTRDIFF_MAX. */ +#include <stdint.h> + +/* The type 'idx_t' holds an (array) index or an (object) size. + Its implementation promotes to a signed integer type, + which can hold the values + 0..2^63-1 (on 64-bit platforms) or + 0..2^31-1 (on 32-bit platforms). + + Why a signed integer type? + + * Security: Signed types can be checked for overflow via + '-fsanitize=undefined', but unsigned types cannot. + + * Comparisons without surprises: ISO C99 § 6.3.1.8 specifies a few + surprising results for comparisons, such as + + (int) -3 < (unsigned long) 7 => false + (int) -3 < (unsigned int) 7 => false + and on 32-bit machines: + (long) -3 < (unsigned int) 7 => false + + This is surprising because the natural comparison order is by + value in the realm of infinite-precision signed integers (ℤ). + + The best way to get rid of such surprises is to use signed types + for numerical integer values, and use unsigned types only for + bit masks and enums. + + Why not use 'size_t' directly? + + * Because 'size_t' is an unsigned type, and a signed type is better. + See above. + + Why not use 'ptrdiff_t' directly? + + * Maintainability: When reading and modifying code, it helps to know that + a certain variable cannot have negative values. For example, when you + have a loop + + int n = ...; + for (int i = 0; i < n; i++) ... + + or + + ptrdiff_t n = ...; + for (ptrdiff_t i = 0; i < n; i++) ... + + you have to ask yourself "what if n < 0?". Whereas in + + idx_t n = ...; + for (idx_t i = 0; i < n; i++) ... + + you know that this case cannot happen. + + Similarly, when a programmer writes + + idx_t = ptr2 - ptr1; + + there is an implied assertion that ptr1 and ptr2 point into the same + object and that ptr1 <= ptr2. + + * Being future-proof: In the future, range types (integers which are + constrained to a certain range of values) may be added to C compilers + or to the C standard. Several programming languages (Ada, Haskell, + Common Lisp, Pascal) already have range types. Such range types may + help producing good code and good warnings. The type 'idx_t' could + then be typedef'ed to a range type that is signed after promotion. */ + +/* In the future, idx_t could be typedef'ed to a signed range type. + The clang "extended integer types", supported in Clang 11 or newer + <https://clang.llvm.org/docs/LanguageExtensions.html#extended-integer-types>, + are a special case of range types. However, these types don't support binary + operators with plain integer types (e.g. expressions such as x > 1). + Therefore, they don't behave like signed types (and not like unsigned types + either). So, we cannot use them here. */ + +/* Use the signed type 'ptrdiff_t'. */ +/* Note: ISO C does not mandate that 'size_t' and 'ptrdiff_t' have the same + size, but it is so on all platforms we have seen since 1990. */ +typedef ptrdiff_t idx_t; + +/* IDX_MAX is the maximum value of an idx_t. */ +#define IDX_MAX PTRDIFF_MAX + +/* So far no need has been found for an IDX_WIDTH macro. + Perhaps there should be another macro IDX_VALUE_BITS that does not + count the sign bit and is therefore one less than PTRDIFF_WIDTH. */ + +#endif /* _IDX_H */ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 66685f4526..4b3bd10a9d 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -35,14 +35,13 @@ #include <string.h> #include <sys/stat.h> #include <unistd.h> +#include <idx.h> #include <scratch_buffer.h> #ifdef _LIBC # include <eloop-threshold.h> # include <shlib-compat.h> -typedef ptrdiff_t idx_t; -# define IDX_MAX PTRDIFF_MAX # define FILE_SYSTEM_PREFIX_LEN(name) 0 # define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name)) # define ISSLASH(c) ((c) == '/') @@ -50,7 +49,6 @@ typedef ptrdiff_t idx_t; #else # define __canonicalize_file_name canonicalize_file_name # define __realpath realpath -# include "idx.h" # include "pathmax.h" # include "filename.h" # if defined _WIN32 && !defined __CYGWIN__ @@ -92,7 +90,6 @@ typedef ptrdiff_t idx_t; #endif #if !FUNC_REALPATH_WORKS || defined _LIBC - static idx_t get_path_max (void) { -- 2.25.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella @ 2020-12-24 22:53 ` Paul Eggert 2020-12-25 20:34 ` Florian Weimer 2020-12-31 23:12 ` Joseph Myers 2 siblings, 0 replies; 22+ messages in thread From: Paul Eggert @ 2020-12-24 22:53 UTC (permalink / raw) To: Adhemerval Zanella, libc-alpha; +Cc: bug-gnulib Thanks, this looks good. Also, the following "[PATCH 3/5] Import filename.h from gnulib" looks good. I merged both changes into Gnulib here: http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=1013adf6bf48627f84a9043cf2631ff13d2a452f ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella 2020-12-24 22:53 ` Paul Eggert @ 2020-12-25 20:34 ` Florian Weimer 2020-12-25 21:38 ` Bruno Haible 2020-12-26 1:29 ` Paul Eggert 2020-12-31 23:12 ` Joseph Myers 2 siblings, 2 replies; 22+ messages in thread From: Florian Weimer @ 2020-12-25 20:34 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha Cc: Paul Eggert, Adhemerval Zanella, bug-gnulib * Adhemerval Zanella via Libc-alpha: > diff --git a/include/idx.h b/include/idx.h > new file mode 100644 > index 0000000000..ad7d31a2bc > --- /dev/null > +++ b/include/idx.h > @@ -0,0 +1,113 @@ > +/* A type for indices and sizes. > + > + Copyright (C) 2020 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 2, or (at your option) > + any later version. > + > + This program 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 General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program; if not, see <https://www.gnu.org/licenses/>. */ This file is linked into the library itself, so we can only accept something that is under a compatible license (probably LGPLv2 or later). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-25 20:34 ` Florian Weimer @ 2020-12-25 21:38 ` Bruno Haible 2020-12-26 1:29 ` Paul Eggert 1 sibling, 0 replies; 22+ messages in thread From: Bruno Haible @ 2020-12-25 21:38 UTC (permalink / raw) To: bug-gnulib; +Cc: Florian Weimer, libc-alpha, Paul Eggert, Adhemerval Zanella Florian Weimer wrote: > > diff --git a/include/idx.h b/include/idx.h > > new file mode 100644 > > index 0000000000..ad7d31a2bc > > --- /dev/null > > +++ b/include/idx.h > > @@ -0,0 +1,113 @@ > > +/* A type for indices and sizes. > > + > > + Copyright (C) 2020 Free Software Foundation, Inc. > > + > > + This program is free software; you can redistribute it and/or modify > > + it under the terms of the GNU General Public License as published by > > + the Free Software Foundation; either version 2, or (at your option) > > + any later version. > > + > > + This program 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 General Public License for more details. > > + > > + You should have received a copy of the GNU General Public License > > + along with this program; if not, see <https://www.gnu.org/licenses/>. */ > > This file is linked into the library itself, so we can only accept > something that is under a compatible license (probably LGPLv2 or later). The file is actually under LGPLv2+ [1][2]. You can replace the license header with an LGPLv2+ header. Bruno [1] https://www.gnu.org/software/gnulib/manual/html_node/Gnulib-licensing.html [2] https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=modules/idx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-25 20:34 ` Florian Weimer 2020-12-25 21:38 ` Bruno Haible @ 2020-12-26 1:29 ` Paul Eggert 1 sibling, 0 replies; 22+ messages in thread From: Paul Eggert @ 2020-12-26 1:29 UTC (permalink / raw) To: Florian Weimer Cc: Adhemerval Zanella, bug-gnulib, Adhemerval Zanella via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 327 bytes --] On 12/25/20 12:34 PM, Florian Weimer wrote: > This file is linked into the library itself, so we can only accept > something that is under a compatible license (probably LGPLv2 or later). I installed the attached patch to the Gnulib copy of idx.h to do that. This uses the same wording as what's in glibc's scratch_buffer.h. [-- Attachment #2: 0001-idx-change-idx.h-comment-to-LGPLv2.1.patch --] [-- Type: text/x-patch, Size: 2545 bytes --] From e0ea25e27e1f1ed70b5dcc2338b9ede365f9a79a Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Fri, 25 Dec 2020 17:26:51 -0800 Subject: [PATCH] idx: change idx.h comment to LGPLv2.1+ * lib/idx.h: Change license notice to match what should be in glibc. gnulib-tool will change it as appropriate anyway, so this is just to simplify syncing with glibc. --- ChangeLog | 7 +++++++ lib/idx.h | 21 +++++++++++---------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 16112f73e..82362718d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2020-12-25 Paul Eggert <eggert@cs.ucla.edu> + + idx: change idx.h comment to LGPLv2.1+ + * lib/idx.h: Change license notice to match what should be in glibc. + gnulib-tool will change it as appropriate anyway, so this is just + to simplify syncing with glibc. + 2020-12-25 Thien-Thi Nguyen <ttn@gnu.org> MODULES.html.sh: Update after 2020-12-19 change. diff --git a/lib/idx.h b/lib/idx.h index ad7d31a2b..024b44ae9 100644 --- a/lib/idx.h +++ b/lib/idx.h @@ -1,19 +1,20 @@ /* A type for indices and sizes. - Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2, or (at your option) - any later version. + 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. - This program is distributed in the hope that it will be useful, + 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 General Public License for more details. + 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 General Public License - along with this program; if not, see <https://www.gnu.org/licenses/>. */ + 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/>. */ #ifndef _IDX_H #define _IDX_H -- 2.27.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella 2020-12-24 22:53 ` Paul Eggert 2020-12-25 20:34 ` Florian Weimer @ 2020-12-31 23:12 ` Joseph Myers 2021-01-01 3:24 ` Paul Eggert 2 siblings, 1 reply; 22+ messages in thread From: Joseph Myers @ 2020-12-31 23:12 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert, bug-gnulib On Thu, 24 Dec 2020, Adhemerval Zanella via Libc-alpha wrote: > + Copyright (C) 2020 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 2, or (at your option) > + any later version. Anything used in glibc libraries should be LGPLv2.1+, not GPL. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] Import idx.h from gnulib 2020-12-31 23:12 ` Joseph Myers @ 2021-01-01 3:24 ` Paul Eggert 0 siblings, 0 replies; 22+ messages in thread From: Paul Eggert @ 2021-01-01 3:24 UTC (permalink / raw) To: Joseph Myers, Adhemerval Zanella; +Cc: libc-alpha, bug-gnulib On 12/31/20 3:12 PM, Joseph Myers wrote: > Anything used in glibc libraries should be LGPLv2.1+, not GPL. That was fixed in the most recently-published version of the patch: https://sourceware.org/pipermail/libc-alpha/2020-December/121178.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] Import filename.h from gnulib 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella 2020-12-24 15:16 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella @ 2020-12-24 15:16 ` Adhemerval Zanella 2020-12-31 23:13 ` Joseph Myers 2020-12-24 15:17 ` [PATCH 4/5] stdlib: Add testcase fro BZ #26241 Adhemerval Zanella 2020-12-24 15:17 ` [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] Adhemerval Zanella 4 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-24 15:16 UTC (permalink / raw) To: libc-alpha, Paul Eggert; +Cc: bug-gnulib And use to simplify stdlib/canonicalize.c implementation. --- include/filename.h | 110 ++++++++++++++++++++++++++++++++++++++++++ stdlib/canonicalize.c | 5 +- 2 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 include/filename.h diff --git a/include/filename.h b/include/filename.h new file mode 100644 index 0000000000..4598fb1d63 --- /dev/null +++ b/include/filename.h @@ -0,0 +1,110 @@ +/* Basic filename support macros. + Copyright (C) 2001-2004, 2007-2020 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +/* From Paul Eggert and Jim Meyering. */ + +#ifndef _FILENAME_H +#define _FILENAME_H + +#include <string.h> + +#ifdef __cplusplus +extern "C" { +#endif + + +/* Filename support. + ISSLASH(C) tests whether C is a directory separator + character. + HAS_DEVICE(Filename) tests whether Filename contains a device + specification. + FILE_SYSTEM_PREFIX_LEN(Filename) length of the device specification + at the beginning of Filename, + index of the part consisting of + alternating components and slashes. + FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE + 1 when a non-empty device specification + can be followed by an empty or relative + part, + 0 when a non-empty device specification + must be followed by a slash, + 0 when device specification don't exist. + IS_ABSOLUTE_FILE_NAME(Filename) + tests whether Filename is independent of + any notion of "current directory". + IS_RELATIVE_FILE_NAME(Filename) + tests whether Filename may be concatenated + to a directory filename. + Note: On native Windows, OS/2, DOS, "c:" is neither an absolute nor a + relative file name! + IS_FILE_NAME_WITH_DIR(Filename) tests whether Filename contains a device + or directory specification. + */ +#if defined _WIN32 || defined __CYGWIN__ \ + || defined __EMX__ || defined __MSDOS__ || defined __DJGPP__ + /* Native Windows, Cygwin, OS/2, DOS */ +# define ISSLASH(C) ((C) == '/' || (C) == '\\') + /* Internal macro: Tests whether a character is a drive letter. */ +# define _IS_DRIVE_LETTER(C) \ + (((C) >= 'A' && (C) <= 'Z') || ((C) >= 'a' && (C) <= 'z')) + /* Help the compiler optimizing it. This assumes ASCII. */ +# undef _IS_DRIVE_LETTER +# define _IS_DRIVE_LETTER(C) \ + (((unsigned int) (C) | ('a' - 'A')) - 'a' <= 'z' - 'a') +# define HAS_DEVICE(Filename) \ + (_IS_DRIVE_LETTER ((Filename)[0]) && (Filename)[1] == ':') +# define FILE_SYSTEM_PREFIX_LEN(Filename) (HAS_DEVICE (Filename) ? 2 : 0) +# ifdef __CYGWIN__ +# define FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE 0 +# else + /* On native Windows, OS/2, DOS, the system has the notion of a + "current directory" on each drive. */ +# define FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE 1 +# endif +# if FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE +# define IS_ABSOLUTE_FILE_NAME(Filename) \ + ISSLASH ((Filename)[FILE_SYSTEM_PREFIX_LEN (Filename)]) +# else +# define IS_ABSOLUTE_FILE_NAME(Filename) \ + (ISSLASH ((Filename)[0]) || HAS_DEVICE (Filename)) +# endif +# define IS_RELATIVE_FILE_NAME(Filename) \ + (! (ISSLASH ((Filename)[0]) || HAS_DEVICE (Filename))) +# define IS_FILE_NAME_WITH_DIR(Filename) \ + (strchr ((Filename), '/') != NULL || strchr ((Filename), '\\') != NULL \ + || HAS_DEVICE (Filename)) +#else + /* Unix */ +# define ISSLASH(C) ((C) == '/') +# define HAS_DEVICE(Filename) ((void) (Filename), 0) +# define FILE_SYSTEM_PREFIX_LEN(Filename) ((void) (Filename), 0) +# define FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE 0 +# define IS_ABSOLUTE_FILE_NAME(Filename) ISSLASH ((Filename)[0]) +# define IS_RELATIVE_FILE_NAME(Filename) (! ISSLASH ((Filename)[0])) +# define IS_FILE_NAME_WITH_DIR(Filename) (strchr ((Filename), '/') != NULL) +#endif + +/* Deprecated macros. For backward compatibility with old users of the + 'filename' module. */ +#define IS_ABSOLUTE_PATH IS_ABSOLUTE_FILE_NAME +#define IS_PATH_WITH_DIR IS_FILE_NAME_WITH_DIR + + +#ifdef __cplusplus +} +#endif + +#endif /* _FILENAME_H */ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 4b3bd10a9d..9111d92a4c 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -36,21 +36,18 @@ #include <sys/stat.h> #include <unistd.h> #include <idx.h> +#include <filename.h> #include <scratch_buffer.h> #ifdef _LIBC # include <eloop-threshold.h> # include <shlib-compat.h> -# define FILE_SYSTEM_PREFIX_LEN(name) 0 -# define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name)) -# define ISSLASH(c) ((c) == '/') # define FUNC_REALPATH_WORKS 1 #else # define __canonicalize_file_name canonicalize_file_name # define __realpath realpath # include "pathmax.h" -# include "filename.h" # if defined _WIN32 && !defined __CYGWIN__ # define __getcwd _getcwd # elif HAVE_GETCWD -- 2.25.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] Import filename.h from gnulib 2020-12-24 15:16 ` [PATCH 3/5] Import filename.h " Adhemerval Zanella @ 2020-12-31 23:13 ` Joseph Myers 2021-01-01 3:31 ` Paul Eggert 0 siblings, 1 reply; 22+ messages in thread From: Joseph Myers @ 2020-12-31 23:13 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert, bug-gnulib On Thu, 24 Dec 2020, Adhemerval Zanella via Libc-alpha wrote: > diff --git a/include/filename.h b/include/filename.h > new file mode 100644 > index 0000000000..4598fb1d63 > --- /dev/null > +++ b/include/filename.h > @@ -0,0 +1,110 @@ > +/* Basic filename support macros. > + Copyright (C) 2001-2004, 2007-2020 Free Software Foundation, Inc. > + > + This program is free software: you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. Anything used in glibc libraries should be LGPLv2.1+. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] Import filename.h from gnulib 2020-12-31 23:13 ` Joseph Myers @ 2021-01-01 3:31 ` Paul Eggert 0 siblings, 0 replies; 22+ messages in thread From: Paul Eggert @ 2021-01-01 3:31 UTC (permalink / raw) To: Joseph Myers, Adhemerval Zanella; +Cc: libc-alpha, bug-gnulib [-- Attachment #1: Type: text/plain, Size: 233 bytes --] On 12/31/20 3:13 PM, Joseph Myers wrote: > Anything used in glibc libraries should be LGPLv2.1+. Thanks for catching that. I installed the attached patch into Gnulib so that glibc and Gnulib copies of filename.h can be identical. [-- Attachment #2: 0001-filename-change-filename.h-comment-to-LGPLv2.1.patch --] [-- Type: text/x-patch, Size: 2662 bytes --] From ec51ca6c9d59037df66afe0804394f66a2a8ff54 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 31 Dec 2020 19:29:08 -0800 Subject: [PATCH] filename: change filename.h comment to LGPLv2.1+ * lib/filename.h: Change license notice to match what should be in glibc, and what modules/filename specifies. This is to simplify syncing with glibc. --- ChangeLog | 5 +++++ lib/filename.h | 20 +++++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index e8b956828..f535c7805 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2020-12-31 Paul Eggert <eggert@cs.ucla.edu> + filename: change filename.h comment to LGPLv2.1+ + * lib/filename.h: Change license notice to match what should be in + glibc, and what modules/filename specifies. This is to simplify + syncing with glibc. + doc: mention year-0 bugs in Solaris etc. * doc/posix-functions/gmtime.texi, doc/posix-functions/gmtime_r.texi: * doc/posix-functions/localtime.texi: diff --git a/lib/filename.h b/lib/filename.h index 4598fb1d6..984154c49 100644 --- a/lib/filename.h +++ b/lib/filename.h @@ -1,18 +1,20 @@ /* Basic filename support macros. Copyright (C) 2001-2004, 2007-2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. - This program is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. + 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. - This program is distributed in the hope that it will be useful, + 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 General Public License for more details. + 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 General Public License - along with this program. If not, see <https://www.gnu.org/licenses/>. */ + 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/>. */ /* From Paul Eggert and Jim Meyering. */ -- 2.27.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] stdlib: Add testcase fro BZ #26241 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella ` (2 preceding siblings ...) 2020-12-24 15:16 ` [PATCH 3/5] Import filename.h " Adhemerval Zanella @ 2020-12-24 15:17 ` Adhemerval Zanella 2020-12-24 15:17 ` [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] Adhemerval Zanella 4 siblings, 0 replies; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-24 15:17 UTC (permalink / raw) To: libc-alpha, Paul Eggert; +Cc: bug-gnulib Checked on x86_64-linux-gnu and i686-linux-gnu. --- stdlib/Makefile | 3 +- stdlib/tst-canon-bz26341.c | 108 ++++++++++++++++++ support/support_set_small_thread_stack_size.c | 12 +- support/xthread.h | 2 + 4 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 stdlib/tst-canon-bz26341.c diff --git a/stdlib/Makefile b/stdlib/Makefile index 29b7cd7071..6518d8993b 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -86,7 +86,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ tst-makecontext-align test-bz22786 tst-strtod-nan-sign \ tst-swapcontext1 tst-setcontext4 tst-setcontext5 \ tst-setcontext6 tst-setcontext7 tst-setcontext8 \ - tst-setcontext9 tst-bz20544 + tst-setcontext9 tst-bz20544 tst-canon-bz26341 tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \ tst-tls-atexit tst-tls-atexit-nodelete @@ -101,6 +101,7 @@ LDLIBS-test-atexit-race = $(shared-thread-library) LDLIBS-test-at_quick_exit-race = $(shared-thread-library) LDLIBS-test-cxa_atexit-race = $(shared-thread-library) LDLIBS-test-on_exit-race = $(shared-thread-library) +LDLIBS-tst-canon-bz26341 = $(shared-thread-library) LDLIBS-test-dlclose-exit-race = $(shared-thread-library) $(libdl) LDFLAGS-test-dlclose-exit-race = $(LDFLAGS-rdynamic) diff --git a/stdlib/tst-canon-bz26341.c b/stdlib/tst-canon-bz26341.c new file mode 100644 index 0000000000..63474bddaa --- /dev/null +++ b/stdlib/tst-canon-bz26341.c @@ -0,0 +1,108 @@ +/* Check if realpath does not consume extra stack space based on symlink + existance in the path (BZ #26341) + 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 <stdlib.h> +#include <string.h> +#include <sys/param.h> + +#include <support/check.h> +#include <support/support.h> +#include <support/temp_file.h> +#include <support/xunistd.h> +#include <support/xthread.h> + +static char *filename; +static size_t filenamelen; +static char *linkname; + +static int +maxsymlinks (void) +{ +#ifdef MAXSYMLINKS + return MAXSYMLINKS; +#else + long int sysconf_symloop_max = sysconf (_SC_SYMLOOP_MAX); + return sysconf_symloop_max <= 0 + ? _POSIX_SYMLOOP_MAX + : sysconf_symloop_max; +#endif +} + +#ifndef PATH_MAX +# define PATH_MAX 1024 +#endif + +static void +create_link (void) +{ + int fd = create_temp_file ("tst-canon-bz26341", &filename); + TEST_VERIFY_EXIT (fd != -1); + xclose (fd); + + char *prevlink = filename; + int maxlinks = maxsymlinks (); + for (int i = 0; i < maxlinks; i++) + { + linkname = xasprintf ("%s%d", filename, i); + xsymlink (prevlink, linkname); + add_temp_file (linkname); + prevlink = linkname; + } + + filenamelen = strlen (filename); +} + +static void * +do_realpath (void *arg) +{ + /* Old implementation of realpath allocates a PATH_MAX using alloca + for each symlink in the path, leading to MAXSYMLINKS times PATH_MAX + maximum stack usage. + This stack allocations tries fill the thread allocated stack minus + both the thread (plus some slack) and the realpath (plus some slack). + If realpath uses more than 2 * PATH_MAX plus some slack it will trigger + a stackoverflow. */ + + const size_t realpath_usage = 2 * PATH_MAX + 1024; + const size_t thread_usage = 1 * PATH_MAX + 1024; + size_t stack_size = support_small_thread_stack_size () + - realpath_usage - thread_usage; + char stack[stack_size]; + char *resolved = stack + stack_size - thread_usage + 1024; + + char *p = realpath (linkname, resolved); + TEST_VERIFY (p != NULL); + TEST_COMPARE_BLOB (resolved, filenamelen, filename, filenamelen); + + return NULL; +} + +static int +do_test (void) +{ + create_link (); + + pthread_t th = xpthread_create (support_small_stack_thread_attribute (), + do_realpath, NULL); + xpthread_join (th); + + return 0; +} + +#include <support/test-driver.c> diff --git a/support/support_set_small_thread_stack_size.c b/support/support_set_small_thread_stack_size.c index 69d66e97db..74a0e38a72 100644 --- a/support/support_set_small_thread_stack_size.c +++ b/support/support_set_small_thread_stack_size.c @@ -20,8 +20,8 @@ #include <pthread.h> #include <support/xthread.h> -void -support_set_small_thread_stack_size (pthread_attr_t *attr) +size_t +support_small_thread_stack_size (void) { /* Some architectures have too small values for PTHREAD_STACK_MIN which cannot be used for creating threads. Ensure that the stack @@ -31,5 +31,11 @@ support_set_small_thread_stack_size (pthread_attr_t *attr) if (stack_size < PTHREAD_STACK_MIN) stack_size = PTHREAD_STACK_MIN; #endif - xpthread_attr_setstacksize (attr, stack_size); + return stack_size; +} + +void +support_set_small_thread_stack_size (pthread_attr_t *attr) +{ + xpthread_attr_setstacksize (attr, support_small_thread_stack_size ()); } diff --git a/support/xthread.h b/support/xthread.h index 05f8d4a7d9..6ba2f5a18b 100644 --- a/support/xthread.h +++ b/support/xthread.h @@ -78,6 +78,8 @@ void xpthread_attr_setguardsize (pthread_attr_t *attr, /* Set the stack size in ATTR to a small value, but still large enough to cover most internal glibc stack usage. */ void support_set_small_thread_stack_size (pthread_attr_t *attr); +/* Return the stack size used on support_set_small_thread_stack_size. */ +size_t support_small_thread_stack_size (void); /* Return a pointer to a thread attribute which requests a small stack. The caller must not free this pointer. */ -- 2.25.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella ` (3 preceding siblings ...) 2020-12-24 15:17 ` [PATCH 4/5] stdlib: Add testcase fro BZ #26241 Adhemerval Zanella @ 2020-12-24 15:17 ` Adhemerval Zanella 2020-12-25 0:27 ` Paul Eggert 4 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-24 15:17 UTC (permalink / raw) To: libc-alpha, Paul Eggert; +Cc: bug-gnulib 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. It also fixes the stdlib/test-canon issued from the gnulib sync. Checked on x86_64-linux-gnu. --- include/scratch_buffer.h | 21 ++++++ stdlib/canonicalize.c | 139 +++++++++++++++++++++++---------------- 2 files changed, 102 insertions(+), 58 deletions(-) diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h index c39da78629..7ae77e5160 100644 --- a/include/scratch_buffer.h +++ b/include/scratch_buffer.h @@ -132,4 +132,25 @@ scratch_buffer_set_array_size (struct scratch_buffer *buffer, (buffer, nelem, size)); } +/* Check if BUFFER is using the internal buffer. */ +static __always_inline bool +scratch_buffer_using_internal (struct scratch_buffer *buffer) +{ + return buffer->data == buffer->__space.__c; +} + +/* Return the internal buffer from BUFFER if it is dynamic allocated, + otherwise returns NULL. Initializes the BUFFER if the internal + dynamic buffer is returned. */ +static __always_inline void * +scratch_buffer_take_buffer (struct scratch_buffer *buffer) +{ + if (scratch_buffer_using_internal (buffer)) + return NULL; + + void *r = buffer->data; + scratch_buffer_init (buffer); + return r; +} + #endif /* _SCRATCH_BUFFER_H */ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 9111d92a4c..78a06227d2 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -88,6 +88,21 @@ #if !FUNC_REALPATH_WORKS || defined _LIBC static idx_t +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf) +{ + ssize_t r; + while (true) + { + ptrdiff_t bufsize = buf->length; + r = __readlink (path, buf->data, bufsize - 1); + if (r < bufsize - 1) + break; + if (!scratch_buffer_grow (buf)) + return -1; + } + return r; +} +static idx_t get_path_max (void) { # ifdef PATH_MAX @@ -144,12 +159,10 @@ __realpath (const char *name, char *resolved) struct scratch_buffer extra_buffer, link_buffer; struct scratch_buffer rname_buffer; - struct scratch_buffer *rname_buf = &rname_buffer; scratch_buffer_init (&extra_buffer); scratch_buffer_init (&link_buffer); - scratch_buffer_init (rname_buf); - char *rname_on_stack = rname_buf->data; - char *rname = rname_on_stack; + scratch_buffer_init (&rname_buffer); + char *rname = rname_buffer.data; bool end_in_extra_buffer = false; bool failed = true; @@ -159,16 +172,16 @@ __realpath (const char *name, char *resolved) if (!IS_ABSOLUTE_FILE_NAME (name)) { - while (!__getcwd (rname, rname_buf->length)) + while (!__getcwd (rname, rname_buffer.length)) { if (errno != ERANGE) { dest = rname; goto error; } - if (!scratch_buffer_grow (rname_buf)) + if (!scratch_buffer_grow (&rname_buffer)) goto error_nomem; - rname = rname_buf->data; + rname = rname_buffer.data; } dest = __rawmemchr (rname, '\0'); start = name; @@ -188,7 +201,7 @@ __realpath (const char *name, char *resolved) start = name + prefix_len; } - for ( ; *start; start = end) + for (end = start ; *start; start = end) { /* Skip sequence of multiple file name separators. */ while (ISSLASH (*start)) @@ -206,6 +219,20 @@ __realpath (const char *name, char *resolved) /* nothing */; else if (startlen == 2 && start[0] == '.' && start[1] == '.') { + if (!ISSLASH (dest[-1])) + *dest++ = '/'; + *dest = '\0'; + + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); + if (n < 0) + { + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno == ENOMEM) + goto error_nomem; + if (errno != EINVAL) + goto error; + } /* Back up to previous component, ignore if at root already. */ if (dest > rname + prefix_len + 1) for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest) @@ -220,46 +247,36 @@ __realpath (const char *name, char *resolved) if (!ISSLASH (dest[-1])) *dest++ = '/'; - while (rname + rname_buf->length - dest <= startlen) + while (rname + rname_buffer.length - dest <= startlen) { idx_t dest_offset = dest - rname; - if (!scratch_buffer_grow_preserve (rname_buf)) + if (!scratch_buffer_grow_preserve (&rname_buffer)) goto error_nomem; - rname = rname_buf->data; + rname = rname_buffer.data; dest = rname + dest_offset; } dest = __mempcpy (dest, start, startlen); *dest = '\0'; - /* If STARTLEN == 0, RNAME ends in '/'; use stat rather than - readlink, because readlink might fail with EINVAL without - checking whether RNAME sans '/' is valid. */ - struct stat st; - char *buf = NULL; - ssize_t n; - if (startlen != 0) + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); + if (n < 0) { - while (true) - { - buf = link_buffer.data; - idx_t bufsize = link_buffer.length; - n = __readlink (rname, buf, bufsize - 1); - if (n < bufsize - 1) - break; - if (!scratch_buffer_grow (&link_buffer)) - goto error_nomem; - } - if (n < 0) - buf = NULL; + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno == ENOMEM) + goto error_nomem; + if (errno != EINVAL) + goto error; } - if (buf) + else { if (++num_links > __eloop_threshold ()) { __set_errno (ELOOP); goto error; } + char *buf = (char*) link_buffer.data; buf[n] = '\0'; @@ -279,7 +296,7 @@ __realpath (const char *name, char *resolved) /* Careful here, end may be a pointer into extra_buf... */ memmove (&extra_buf[n], end, len + 1); - name = end = memcpy (extra_buf, buf, n); + name = end = memcpy (extra_buf, link_buffer.data, n); end_in_extra_buffer = true; if (IS_ABSOLUTE_FILE_NAME (buf)) @@ -309,10 +326,6 @@ __realpath (const char *name, char *resolved) dest++; } } - else if (! (startlen == 0 - ? stat (rname, &st) == 0 || errno == EOVERFLOW - : errno == EINVAL)) - goto error; } } if (dest > rname + prefix_len + 1 && ISSLASH (dest[-1])) @@ -320,34 +333,44 @@ __realpath (const char *name, char *resolved) if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len && ISSLASH (*dest) && !ISSLASH (dest[1])) dest++; + *dest = '\0'; failed = false; error: - *dest++ = '\0'; - if (resolved != NULL && dest - rname <= get_path_max ()) - rname = strcpy (resolved, rname); + if (resolved != NULL) + { + if (dest - rname <= get_path_max ()) + rname = strcpy (resolved, rname); + } + else + { + if (rname == resolved) + return rname; + + idx_t rname_size = dest - rname; + if (scratch_buffer_using_internal (&rname_buffer)) + { + rname = malloc (rname_size + 1); + if (rname != NULL) + { + memcpy (rname, rname_buffer.data, rname_size); + rname[rname_size] = '\0'; + } + } + else + { + rname = scratch_buffer_take_buffer (&rname_buffer); + char *result = realloc (rname, rname_size); + if (result != NULL) + rname = result; + } + } error_nomem: - scratch_buffer_free (&extra_buffer); scratch_buffer_free (&link_buffer); - if (failed || rname == resolved) - scratch_buffer_free (rname_buf); - - if (failed) - return NULL; - - if (rname == resolved) - return rname; - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) - { - rname = malloc (rname_size); - if (rname == NULL) - return NULL; - return memcpy (rname, rname_on_stack, rname_size); - } - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + scratch_buffer_free (&extra_buffer); + scratch_buffer_free (&rname_buffer); + return failed ? NULL : rname; } libc_hidden_def (__realpath) versioned_symbol (libc, __realpath, realpath, GLIBC_2_3); -- 2.25.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] 2020-12-24 15:17 ` [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] Adhemerval Zanella @ 2020-12-25 0:27 ` Paul Eggert 2020-12-28 11:42 ` Adhemerval Zanella 0 siblings, 1 reply; 22+ messages in thread From: Paul Eggert @ 2020-12-25 0:27 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: bug-gnulib, libc-alpha [-- Attachment #1: Type: text/plain, Size: 4676 bytes --] This email finishes the review of this proposed glibc patchset. (I didn't look at patch 4/5 for test cases.) On 12/24/20 7:17 AM, Adhemerval Zanella wrote: > +/* Check if BUFFER is using the internal buffer. */ > +static __always_inline bool > +scratch_buffer_using_internal (struct scratch_buffer *buffer) > +{ > + return buffer->data == buffer->__space.__c; > +} > + > +/* Return the internal buffer from BUFFER if it is dynamic allocated, > + otherwise returns NULL. Initializes the BUFFER if the internal > + dynamic buffer is returned. */ > +static __always_inline void * > +scratch_buffer_take_buffer (struct scratch_buffer *buffer) > +{ > + if (scratch_buffer_using_internal (buffer)) > + return NULL; > + > + void *r = buffer->data; > + scratch_buffer_init (buffer); > + return r; > +} This combination of functions is a little tricky. Instead, how about a single function that duplicates the scratch buffer on the heap, and frees the scratch buffer? Please see the attached proposed patch for Gnulib, which implements this idea. (I have not installed this into Gnulib.) Also, shouldn't we merge the already-existing Gnulib scratch_buffer changes into glibc, along with this new change? > static idx_t > +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf) > +{ > + ssize_t r; > + while (true) > + { > + ptrdiff_t bufsize = buf->length; > + r = __readlink (path, buf->data, bufsize - 1); > + if (r < bufsize - 1) > + break; > + if (!scratch_buffer_grow (buf)) > + return -1; > + } > + return r; > +} This function seems to exist because the proposed code calls readlink in two places. Current gnulib has been changed to call it in just one place, so there's less need to split out the function (the splitting complicates out-of-memory checking). > - scratch_buffer_init (rname_buf); > - char *rname_on_stack = rname_buf->data; > ... > + scratch_buffer_init (&rname_buffer); > + char *rname = rname_buffer.data; Doesn't this sort of thing have the potential to run into GCC bug 93644? That bug tends to be flaky; change platforms, or a few lines of code, and the problem recurs. Although it's just a bogus warning it cannot be turned off Gnulib has a GCC_LINT fix for this, which glibc could use simply with "#define GCC_LINT 1" in the "#ifdef _GLIBC" code. > @@ -206,6 +219,20 @@ __realpath (const char *name, char *resolved) > /* nothing */; > else if (startlen == 2 && start[0] == '.' && start[1] == '.') > { > + if (!ISSLASH (dest[-1])) > + *dest++ = '/'; > + *dest = '\0'; > + > + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); > + if (n < 0) > + { > + if (errno == ENOTDIR && dest[-1] == '/') > + dest[-1] = '\0'; > + if (errno == ENOMEM) > + goto error_nomem; > + if (errno != EINVAL) > + goto error; > + } This can call readlink twice, once with trailing slash and once without. Better to call it just once. > + char *buf = (char*) link_buffer.data; > > buf[n] = '\0'; > > @@ -279,7 +296,7 @@ __realpath (const char *name, char *resolved) > > /* Careful here, end may be a pointer into extra_buf... */ > memmove (&extra_buf[n], end, len + 1); > - name = end = memcpy (extra_buf, buf, n); > + name = end = memcpy (extra_buf, link_buffer.data, n); If buf already equals link_buffer.data, no need for the patch to change buf to link_buffer.data. > - else if (! (startlen == 0 > - ? stat (rname, &st) == 0 || errno == EOVERFLOW > - : errno == EINVAL)) > - goto error; I think current Gnulib addresses this issue in a different way, that doesn't involve the extra readlink calls. > if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len > && ISSLASH (*dest) && !ISSLASH (dest[1])) > dest++; > + *dest = '\0'; > failed = false; > > error: > - *dest++ = '\0'; This looks dubious, as the error case also needs *dest to be '\0' and to increment dest (for when returning NULL when resolved != NULL). Proposed patch to Gnulib attached. I hope this patch (along with what's already in Gnulib) addresses all the issues raised in your glibc patches, in the sense that the relevant files can be identical in Glibc and in Gnulib. I haven't installed this into Gnulib master on savannah. [-- Attachment #2: 0001-canonicalize-simplify-via-scratch_buffer_dupfree.patch --] [-- Type: text/x-patch, Size: 7665 bytes --] From 5186fb6c9e2840ab921418bf73fe2662e200f89d Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 24 Dec 2020 16:17:44 -0800 Subject: [PATCH] canonicalize: simplify via scratch_buffer_dupfree * config/srclist.txt: Adjust accordingly. * lib/canonicalize-lgpl.c (realpath_stk): * lib/canonicalize.c (canonicalize_filename_mode_stk): Simplify by using scratch_buffer_dupfree. * lib/malloc/scratch_buffer.h (scratch_buffer_dupfree): New function. * lib/malloc/scratch_buffer_dupfree.c: New file. * modules/scratch_buffer (Files, Depends-on): Add malloc/scratch_buffer_dupfree.c. --- ChangeLog | 10 +++++++ config/srclist.txt | 3 ++- lib/canonicalize-lgpl.c | 20 ++++---------- lib/canonicalize.c | 9 +++---- lib/malloc/scratch_buffer.h | 16 +++++++++++ lib/malloc/scratch_buffer_dupfree.c | 41 +++++++++++++++++++++++++++++ modules/scratch_buffer | 4 ++- 7 files changed, 81 insertions(+), 22 deletions(-) create mode 100644 lib/malloc/scratch_buffer_dupfree.c diff --git a/ChangeLog b/ChangeLog index 30be929b5..6b3ca8b5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2020-12-24 Paul Eggert <eggert@cs.ucla.edu> + canonicalize: simplify via scratch_buffer_dupfree + * config/srclist.txt: Adjust accordingly. + * lib/canonicalize-lgpl.c (realpath_stk): + * lib/canonicalize.c (canonicalize_filename_mode_stk): + Simplify by using scratch_buffer_dupfree. + * lib/malloc/scratch_buffer.h (scratch_buffer_dupfree): New function. + * lib/malloc/scratch_buffer_dupfree.c: New file. + * modules/scratch_buffer (Files, Depends-on): + Add malloc/scratch_buffer_dupfree.c. + canonicalize-lgpl: merge proposed libc changes This merges the changes proposed for glibc in: https://sourceware.org/pipermail/libc-alpha/2020-December/121085.html diff --git a/config/srclist.txt b/config/srclist.txt index f33b1353f..3956082c8 100644 --- a/config/srclist.txt +++ b/config/srclist.txt @@ -49,7 +49,8 @@ $GNUORG Copyright/request-assign.future doc/Copyright $GNUORG Copyright/request-assign.program doc/Copyright $GNUORG Copyright/request-disclaim.changes doc/Copyright -$LIBCSRC include/scratch_buffer.h lib/malloc +#$LIBCSRC include/scratch_buffer.h lib/malloc +#$LIBCSRC malloc/scratch_buffer_dupfree.c lib/malloc $LIBCSRC malloc/scratch_buffer_grow.c lib/malloc $LIBCSRC malloc/scratch_buffer_grow_preserve.c lib/malloc $LIBCSRC malloc/scratch_buffer_set_array_size.c lib/malloc diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 8c8f98abc..b5f51122b 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -404,24 +404,14 @@ error: error_nomem: scratch_buffer_free (&extra_buffer); scratch_buffer_free (&link_buffer); - if (failed || rname == resolved) - scratch_buffer_free (rname_buf); - - if (failed) - return NULL; - if (rname == resolved) - return rname; - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) + if (failed || rname == resolved) { - rname = malloc (rname_size); - if (rname == NULL) - return NULL; - return memcpy (rname, rname_on_stack, rname_size); + scratch_buffer_free (rname_buf); + return failed ? NULL : resolved; } - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + + return scratch_buffer_dupfree (rname_buf, dest - rname); } /* Return the canonical absolute name of file NAME. A canonical name diff --git a/lib/canonicalize.c b/lib/canonicalize.c index 2c88335bf..366b658a2 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -463,11 +463,10 @@ error: return NULL; } - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) - return xmemdup (rname, rname_size); - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + char *result = scratch_buffer_dupfree (rname_buf, dest - rname); + if (!result) + xalloc_die (); + return result; } /* Return the canonical absolute name of file NAME, while treating diff --git a/lib/malloc/scratch_buffer.h b/lib/malloc/scratch_buffer.h index c39da7862..48d651b41 100644 --- a/lib/malloc/scratch_buffer.h +++ b/lib/malloc/scratch_buffer.h @@ -132,4 +132,20 @@ scratch_buffer_set_array_size (struct scratch_buffer *buffer, (buffer, nelem, size)); } +/* Return a copy of *BUFFER's first SIZE bytes as a heap-allocated block, + deallocating *BUFFER if it was heap-allocated. SIZE must be at + most *BUFFER's size. Return NULL (setting errno) on memory + exhaustion. */ +void *__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer, + size_t size); +libc_hidden_proto (__libc_scratch_buffer_dupfree) + +/* Alias for __libc_scratch_dupfree. */ +static __always_inline void * +scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size) +{ + void *r = __libc_scratch_buffer_dupfree (buffer, size); + return __glibc_likely (r != NULL) ? r : NULL; +} + #endif /* _SCRATCH_BUFFER_H */ diff --git a/lib/malloc/scratch_buffer_dupfree.c b/lib/malloc/scratch_buffer_dupfree.c new file mode 100644 index 000000000..5561e99b0 --- /dev/null +++ b/lib/malloc/scratch_buffer_dupfree.c @@ -0,0 +1,41 @@ +/* Variable-sized buffer with on-stack default allocation. + 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/>. */ + +#ifndef _LIBC +# include <libc-config.h> +#endif + +#include <scratch_buffer.h> +#include <string.h> + +void * +__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size) +{ + void *data = buffer->data; + if (data == buffer->__space.__c) + { + void *copy = malloc (size); + return copy != NULL ? memcpy (copy, data, size) : NULL; + } + else + { + void *copy = realloc (data, size); + return copy != NULL ? copy : data; + } +} +libc_hidden_def (__libc_scratch_buffer_dupfree) diff --git a/modules/scratch_buffer b/modules/scratch_buffer index 4f9a72581..7eedae7cc 100644 --- a/modules/scratch_buffer +++ b/modules/scratch_buffer @@ -4,6 +4,7 @@ Variable-sized buffer with on-stack default allocation. Files: lib/scratch_buffer.h lib/malloc/scratch_buffer.h +lib/malloc/scratch_buffer_dupfree.c lib/malloc/scratch_buffer_grow.c lib/malloc/scratch_buffer_grow_preserve.c lib/malloc/scratch_buffer_set_array_size.c @@ -17,7 +18,8 @@ stddef configure.ac: Makefile.am: -lib_SOURCES += malloc/scratch_buffer_grow.c \ +lib_SOURCES += malloc/scratch_buffer_dupfree.c \ + malloc/scratch_buffer_grow.c \ malloc/scratch_buffer_grow_preserve.c \ malloc/scratch_buffer_set_array_size.c -- 2.27.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] 2020-12-25 0:27 ` Paul Eggert @ 2020-12-28 11:42 ` Adhemerval Zanella 2020-12-28 13:32 ` Adhemerval Zanella 2020-12-28 21:04 ` Paul Eggert 0 siblings, 2 replies; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-28 11:42 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib, libc-alpha On 24/12/2020 21:27, Paul Eggert wrote: > This email finishes the review of this proposed glibc patchset. (I didn't look at patch 4/5 for test cases.) > > On 12/24/20 7:17 AM, Adhemerval Zanella wrote: > >> +/* Check if BUFFER is using the internal buffer. */ >> +static __always_inline bool >> +scratch_buffer_using_internal (struct scratch_buffer *buffer) >> +{ >> + return buffer->data == buffer->__space.__c; >> +} >> + >> +/* Return the internal buffer from BUFFER if it is dynamic allocated, >> + otherwise returns NULL. Initializes the BUFFER if the internal >> + dynamic buffer is returned. */ >> +static __always_inline void * >> +scratch_buffer_take_buffer (struct scratch_buffer *buffer) >> +{ >> + if (scratch_buffer_using_internal (buffer)) >> + return NULL; >> + >> + void *r = buffer->data; >> + scratch_buffer_init (buffer); >> + return r; >> +} > > This combination of functions is a little tricky. Instead, how about a single function that duplicates the scratch buffer on the heap, and frees the scratch buffer? Please see the attached proposed patch for Gnulib, which implements this idea. (I have not installed this into Gnulib.) Indeed this seems a better alternative. > > Also, shouldn't we merge the already-existing Gnulib scratch_buffer changes into glibc, along with this new change? Which changes are you referring? Checking against bbaba6ce5 I see all glibc files for scratch_buffer are similar to the gnulib ones. > >> static idx_t >> +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf) >> +{ >> + ssize_t r; >> + while (true) >> + { >> + ptrdiff_t bufsize = buf->length; >> + r = __readlink (path, buf->data, bufsize - 1); >> + if (r < bufsize - 1) >> + break; >> + if (!scratch_buffer_grow (buf)) >> + return -1; >> + } >> + return r; >> +} > > This function seems to exist because the proposed code calls readlink in two places. Current gnulib has been changed to call it in just one place, so there's less need to split out the function (the splitting complicates out-of-memory checking). Yes, this trades a stat call by an extra readlink call. However it seems that your strategy to use faccessat should be better, assuming it is lighter syscall. > >> - scratch_buffer_init (rname_buf); >> - char *rname_on_stack = rname_buf->data; >> ... >> + scratch_buffer_init (&rname_buffer); >> + char *rname = rname_buffer.data; > > Doesn't this sort of thing have the potential to run into GCC bug 93644? That bug tends to be flaky; change platforms, or a few lines of code, and the problem recurs. Although it's just a bogus warning it cannot be turned off Gnulib has a GCC_LINT fix for this, which glibc could use simply with "#define GCC_LINT 1" in the "#ifdef _GLIBC" code. I wasn't aware on the GCC issue in fact (it seems to affect GCC 10/11 and I am using GCC 9.2.1). > >> @@ -206,6 +219,20 @@ __realpath (const char *name, char *resolved) >> /* nothing */; >> else if (startlen == 2 && start[0] == '.' && start[1] == '.') >> { >> + if (!ISSLASH (dest[-1])) >> + *dest++ = '/'; >> + *dest = '\0'; >> + >> + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); >> + if (n < 0) >> + { >> + if (errno == ENOTDIR && dest[-1] == '/') >> + dest[-1] = '\0'; >> + if (errno == ENOMEM) >> + goto error_nomem; >> + if (errno != EINVAL) >> + goto error; >> + } > > This can call readlink twice, once with trailing slash and once without. Better to call it just once. Right. > >> + char *buf = (char*) link_buffer.data; >> buf[n] = '\0'; >> @@ -279,7 +296,7 @@ __realpath (const char *name, char *resolved) >> /* Careful here, end may be a pointer into extra_buf... */ >> memmove (&extra_buf[n], end, len + 1); >> - name = end = memcpy (extra_buf, buf, n); >> + name = end = memcpy (extra_buf, link_buffer.data, n); > > If buf already equals link_buffer.data, no need for the patch to change buf to link_buffer.data. > Indeed, this is a left over from development. >> - else if (! (startlen == 0 >> - ? stat (rname, &st) == 0 || errno == EOVERFLOW >> - : errno == EINVAL)) >> - goto error; > > I think current Gnulib addresses this issue in a different way, that doesn't involve the extra readlink calls. >> if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len >> && ISSLASH (*dest) && !ISSLASH (dest[1])) Yes, I noted now on gnulib master that you handled it with an faccessat on dir_check. >> dest++; >> + *dest = '\0'; >> failed = false; >> error: >> - *dest++ = '\0'; > > This looks dubious, as the error case also needs *dest to be '\0' and to increment dest (for when returning NULL when resolved != NULL). > Yes, I think this is also a leftover from development. > Proposed patch to Gnulib attached. I hope this patch (along with what's already in Gnulib) addresses all the issues raised in your glibc patches, in the sense that the relevant files can be identical in Glibc and in Gnulib. I haven't installed this into Gnulib master on savannah. Changes seems ok, I will send a v3 with the proposed changes to sync the gnulib headers, along with the scratch_buffer change, the gnulib sync that fix all the issues, and the extra test. Thanks for working on this. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] 2020-12-28 11:42 ` Adhemerval Zanella @ 2020-12-28 13:32 ` Adhemerval Zanella 2020-12-28 21:04 ` Paul Eggert 1 sibling, 0 replies; 22+ messages in thread From: Adhemerval Zanella @ 2020-12-28 13:32 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib, libc-alpha On 28/12/2020 08:42, Adhemerval Zanella wrote: >>> static idx_t >>> +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf) >>> +{ >>> + ssize_t r; >>> + while (true) >>> + { >>> + ptrdiff_t bufsize = buf->length; >>> + r = __readlink (path, buf->data, bufsize - 1); >>> + if (r < bufsize - 1) >>> + break; >>> + if (!scratch_buffer_grow (buf)) >>> + return -1; >>> + } >>> + return r; >>> +} >> >> This function seems to exist because the proposed code calls readlink in two places. Current gnulib has been changed to call it in just one place, so there's less need to split out the function (the splitting complicates out-of-memory checking). > > Yes, this trades a stat call by an extra readlink call. However it seems > that your strategy to use faccessat should be better, assuming it is lighter > syscall. Also, it would require a recent kernel (5.8) to avoid faccessat to fallback to use __NR_faccessat (it would make each call to faccessat2 to issue two syscall). We can live with this and if it turns a performance issue we ca the hack to set a global variable to avoid it. Now this snippet: 47 # include <sysdep.h> 48 # ifdef __ASSUME_FACCESSAT2 49 # define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2 50 # else 51 # define FACCESSAT_NEVER_EOVERFLOWS true 52 # endif is not required, since Linux faccessat fallback uses LFS stat call that does not return EOVERFLOW. And it also bleeds Linux implementation details on generic code. I will send a patch to fix it. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] 2020-12-28 11:42 ` Adhemerval Zanella 2020-12-28 13:32 ` Adhemerval Zanella @ 2020-12-28 21:04 ` Paul Eggert 1 sibling, 0 replies; 22+ messages in thread From: Paul Eggert @ 2020-12-28 21:04 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: bug-gnulib, libc-alpha [-- Attachment #1: Type: text/plain, Size: 735 bytes --] On 12/28/20 3:42 AM, Adhemerval Zanella wrote: > On 24/12/2020 21:27, Paul Eggert wrote: >> Also, shouldn't we merge the already-existing Gnulib scratch_buffer changes into glibc, along with this new change? > > Which changes are you referring? Checking against bbaba6ce5 I see all > glibc files for scratch_buffer are similar to the gnulib ones. Sorry false alarm. Don't know what I was thinking of. I incorporated almost all the changes suggested by your recently-proposed glibc changes into Gnulib, by installing the attached into Gnulib savannah master. There are still a few minor discrepancies from what you proposed for glibc, and I plan to comment on those discrepancies in reply to your other emails. [-- Attachment #2: 0001-canonicalize-simplify-via-scratch_buffer_dupfree.patch --] [-- Type: text/x-patch, Size: 7577 bytes --] From 62c8b7b665b7c4c54d53496407342c5aefc9f8d1 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 28 Dec 2020 11:38:58 -0800 Subject: [PATCH 1/3] canonicalize: simplify via scratch_buffer_dupfree * config/srclist.txt: Adjust accordingly. * lib/canonicalize-lgpl.c (realpath_stk): * lib/canonicalize.c (canonicalize_filename_mode_stk): Simplify by using scratch_buffer_dupfree. * lib/malloc/scratch_buffer.h (scratch_buffer_dupfree): New function. * lib/malloc/scratch_buffer_dupfree.c: New file. * modules/scratch_buffer (Files, Depends-on): Add malloc/scratch_buffer_dupfree.c. --- ChangeLog | 12 +++++++++ config/srclist.txt | 3 ++- lib/canonicalize-lgpl.c | 20 ++++---------- lib/canonicalize.c | 9 +++---- lib/malloc/scratch_buffer.h | 16 +++++++++++ lib/malloc/scratch_buffer_dupfree.c | 41 +++++++++++++++++++++++++++++ modules/scratch_buffer | 4 ++- 7 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 lib/malloc/scratch_buffer_dupfree.c diff --git a/ChangeLog b/ChangeLog index 97966b427..0428619c1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2020-12-28 Paul Eggert <eggert@cs.ucla.edu> + + canonicalize: simplify via scratch_buffer_dupfree + * config/srclist.txt: Adjust accordingly. + * lib/canonicalize-lgpl.c (realpath_stk): + * lib/canonicalize.c (canonicalize_filename_mode_stk): + Simplify by using scratch_buffer_dupfree. + * lib/malloc/scratch_buffer.h (scratch_buffer_dupfree): New function. + * lib/malloc/scratch_buffer_dupfree.c: New file. + * modules/scratch_buffer (Files, Depends-on): + Add malloc/scratch_buffer_dupfree.c. + 2020-12-27 Paul Eggert <eggert@cs.ucla.edu> regex: remove glibc21.m4 diff --git a/config/srclist.txt b/config/srclist.txt index f33b1353f..3956082c8 100644 --- a/config/srclist.txt +++ b/config/srclist.txt @@ -49,7 +49,8 @@ $GNUORG Copyright/request-assign.future doc/Copyright $GNUORG Copyright/request-assign.program doc/Copyright $GNUORG Copyright/request-disclaim.changes doc/Copyright -$LIBCSRC include/scratch_buffer.h lib/malloc +#$LIBCSRC include/scratch_buffer.h lib/malloc +#$LIBCSRC malloc/scratch_buffer_dupfree.c lib/malloc $LIBCSRC malloc/scratch_buffer_grow.c lib/malloc $LIBCSRC malloc/scratch_buffer_grow_preserve.c lib/malloc $LIBCSRC malloc/scratch_buffer_set_array_size.c lib/malloc diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 68b6fdf6e..7ac5d412d 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -413,24 +413,14 @@ error: error_nomem: scratch_buffer_free (&extra_buffer); scratch_buffer_free (&link_buffer); - if (failed || rname == resolved) - scratch_buffer_free (rname_buf); - - if (failed) - return NULL; - if (rname == resolved) - return rname; - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) + if (failed || rname == resolved) { - rname = malloc (rname_size); - if (rname == NULL) - return NULL; - return memcpy (rname, rname_on_stack, rname_size); + scratch_buffer_free (rname_buf); + return failed ? NULL : resolved; } - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + + return scratch_buffer_dupfree (rname_buf, dest - rname); } /* Return the canonical absolute name of file NAME. A canonical name diff --git a/lib/canonicalize.c b/lib/canonicalize.c index e8b74d855..48a49e412 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -469,11 +469,10 @@ error: return NULL; } - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) - return xmemdup (rname, rname_size); - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + char *result = scratch_buffer_dupfree (rname_buf, dest - rname); + if (!result) + xalloc_die (); + return result; } /* Return the canonical absolute name of file NAME, while treating diff --git a/lib/malloc/scratch_buffer.h b/lib/malloc/scratch_buffer.h index c39da7862..48d651b41 100644 --- a/lib/malloc/scratch_buffer.h +++ b/lib/malloc/scratch_buffer.h @@ -132,4 +132,20 @@ scratch_buffer_set_array_size (struct scratch_buffer *buffer, (buffer, nelem, size)); } +/* Return a copy of *BUFFER's first SIZE bytes as a heap-allocated block, + deallocating *BUFFER if it was heap-allocated. SIZE must be at + most *BUFFER's size. Return NULL (setting errno) on memory + exhaustion. */ +void *__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer, + size_t size); +libc_hidden_proto (__libc_scratch_buffer_dupfree) + +/* Alias for __libc_scratch_dupfree. */ +static __always_inline void * +scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size) +{ + void *r = __libc_scratch_buffer_dupfree (buffer, size); + return __glibc_likely (r != NULL) ? r : NULL; +} + #endif /* _SCRATCH_BUFFER_H */ diff --git a/lib/malloc/scratch_buffer_dupfree.c b/lib/malloc/scratch_buffer_dupfree.c new file mode 100644 index 000000000..5561e99b0 --- /dev/null +++ b/lib/malloc/scratch_buffer_dupfree.c @@ -0,0 +1,41 @@ +/* Variable-sized buffer with on-stack default allocation. + 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/>. */ + +#ifndef _LIBC +# include <libc-config.h> +#endif + +#include <scratch_buffer.h> +#include <string.h> + +void * +__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size) +{ + void *data = buffer->data; + if (data == buffer->__space.__c) + { + void *copy = malloc (size); + return copy != NULL ? memcpy (copy, data, size) : NULL; + } + else + { + void *copy = realloc (data, size); + return copy != NULL ? copy : data; + } +} +libc_hidden_def (__libc_scratch_buffer_dupfree) diff --git a/modules/scratch_buffer b/modules/scratch_buffer index 4f9a72581..7eedae7cc 100644 --- a/modules/scratch_buffer +++ b/modules/scratch_buffer @@ -4,6 +4,7 @@ Variable-sized buffer with on-stack default allocation. Files: lib/scratch_buffer.h lib/malloc/scratch_buffer.h +lib/malloc/scratch_buffer_dupfree.c lib/malloc/scratch_buffer_grow.c lib/malloc/scratch_buffer_grow_preserve.c lib/malloc/scratch_buffer_set_array_size.c @@ -17,7 +18,8 @@ stddef configure.ac: Makefile.am: -lib_SOURCES += malloc/scratch_buffer_grow.c \ +lib_SOURCES += malloc/scratch_buffer_dupfree.c \ + malloc/scratch_buffer_grow.c \ malloc/scratch_buffer_grow_preserve.c \ malloc/scratch_buffer_set_array_size.c -- 2.27.0 [-- Attachment #3: 0002-canonicalize-lgpl-accommodate-picky-cpp.patch --] [-- Type: text/x-patch, Size: 1573 bytes --] From 7796b1f6235a0328dc411d51d0da45ddda6d575d Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 28 Dec 2020 11:58:38 -0800 Subject: [PATCH 2/3] canonicalize-lgpl: accommodate picky cpp * lib/canonicalize-lgpl.c: Use "defined FUNC_REALPATH_WORKS" in case preprocessor is picky. Reported by Adhemerval Zanella in: https://sourceware.org/pipermail/libc-alpha/2020-December/121130.html --- ChangeLog | 5 +++++ lib/canonicalize-lgpl.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 0428619c1..9a91eda92 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2020-12-28 Paul Eggert <eggert@cs.ucla.edu> + canonicalize-lgpl: accommodate picky cpp + * lib/canonicalize-lgpl.c: Use "defined FUNC_REALPATH_WORKS" in + case preprocessor is picky. Reported by Adhemerval Zanella in: + https://sourceware.org/pipermail/libc-alpha/2020-December/121130.html + canonicalize: simplify via scratch_buffer_dupfree * config/srclist.txt: Adjust accordingly. * lib/canonicalize-lgpl.c (realpath_stk): diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 7ac5d412d..332b5bab4 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -98,7 +98,7 @@ # define FACCESSAT_NEVER_EOVERFLOWS false #endif -#if !FUNC_REALPATH_WORKS || defined _LIBC +#if defined _LIBC || !FUNC_REALPATH_WORKS /* Return true if FILE's existence can be shown, false (setting errno) otherwise. Follow symbolic links. */ -- 2.27.0 [-- Attachment #4: 0003-faccessat-revert-recent-EOVERFLOW-change.patch --] [-- Type: text/x-patch, Size: 10863 bytes --] From 20527b26f5e020ebaa48a8d5daec4cccebf051df Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 28 Dec 2020 12:38:52 -0800 Subject: [PATCH 3/3] faccessat: revert recent EOVERFLOW change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I misunderstood the glibc source code. Deduced from Adhemerval Zanella’s proposed glibc patch in: https://sourceware.org/pipermail/libc-alpha/2020-December/121131.html * doc/posix-functions/faccessat.texi: It is not a problem. * lib/canonicalize-lgpl.c, lib/canonicalize.c, lib/faccessat.c: (FACCESSAT_NEVER_OVERFLOWS): Remove. All uses removed. * lib/faccessat.c: Revert to simpler version now that LSTAT_FOLLOWS_SLASHED_SYMLINK must be false. * m4/faccessat.m4 (gl_FUNC_FACCESSAT_EOVERFLOW): Remove. All uses removed. * modules/canonicalize, modules/canonicalize-lgpl (Files): Remove m4/faccessat.m4. --- ChangeLog | 14 +++++++++++++ doc/posix-functions/faccessat.texi | 14 +++---------- lib/canonicalize-lgpl.c | 16 ++------------- lib/canonicalize.c | 10 ++------- lib/faccessat.c | 14 +------------ m4/canonicalize.m4 | 4 +--- m4/faccessat.m4 | 33 +++--------------------------- modules/canonicalize | 1 - modules/canonicalize-lgpl | 1 - 9 files changed, 26 insertions(+), 81 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9a91eda92..1481becc9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,19 @@ 2020-12-28 Paul Eggert <eggert@cs.ucla.edu> + faccessat: revert recent EOVERFLOW change + I misunderstood the glibc source code. Deduced from + Adhemerval Zanella’s proposed glibc patch in: + https://sourceware.org/pipermail/libc-alpha/2020-December/121131.html + * doc/posix-functions/faccessat.texi: It is not a problem. + * lib/canonicalize-lgpl.c, lib/canonicalize.c, lib/faccessat.c: + (FACCESSAT_NEVER_OVERFLOWS): Remove. All uses removed. + * lib/faccessat.c: Revert to simpler version now that + LSTAT_FOLLOWS_SLASHED_SYMLINK must be false. + * m4/faccessat.m4 (gl_FUNC_FACCESSAT_EOVERFLOW): + Remove. All uses removed. + * modules/canonicalize, modules/canonicalize-lgpl (Files): + Remove m4/faccessat.m4. + canonicalize-lgpl: accommodate picky cpp * lib/canonicalize-lgpl.c: Use "defined FUNC_REALPATH_WORKS" in case preprocessor is picky. Reported by Adhemerval Zanella in: diff --git a/doc/posix-functions/faccessat.texi b/doc/posix-functions/faccessat.texi index 07ea8e7bf..5d5165e47 100644 --- a/doc/posix-functions/faccessat.texi +++ b/doc/posix-functions/faccessat.texi @@ -15,12 +15,6 @@ glibc 2.3.6, macOS 10.12, FreeBSD 7.4, NetBSD 6.1.5, OpenBSD 4.9, Minix 3.1.8, A On some platforms, @code{faccessat (dfd, "file/", amode, flag)} succeeds instead of failing when @file{file} is not a directory. macOS 10.13. -@item -On some platforms, @code{faccessat} can incorrectly fail with -@code{EOVERFLOW} when the mode is @code{F_OK}: -GNU/Linux with glibc 2.32, or with Linux kernel 5.7. -@c This bug should be fixed in glibc 2.33 and kernel 5.8. See: -@c https://sourceware.org/bugzilla/show_bug.cgi?id=18683 @end itemize Portability problems not fixed by Gnulib: @@ -36,11 +30,9 @@ The replacement does not support the @code{AT_SYMLINK_NOFOLLOW} flag, which is supported by GNU @code{faccessat}. @item On some platforms, @code{faccessat} can mishandle @code{AT_EACCESS} -after a process starts as root and then becomes non-root, -or can incorrectly fail with @code{EOVERFLOW} when the mode -is not @code{F_OK}: -GNU/Linux with glibc 2.32, or with Linux kernel 5.7. -@c These bugs should be fixed in glibc 2.33 and kernel 5.8. See: +after a process starts as root and then becomes non-root: +GNU/Linux with glibc 2.32. +@c This bug should be fixed in glibc 2.33. See: @c https://sourceware.org/bugzilla/show_bug.cgi?id=18683 @end itemize diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 332b5bab4..04fe95253 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -44,12 +44,6 @@ #ifdef _LIBC # include <shlib-compat.h> -# include <sysdep.h> -# ifdef __ASSUME_FACCESSAT2 -# define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2 -# else -# define FACCESSAT_NEVER_EOVERFLOWS true -# endif # define GCC_LINT 1 # define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__)) #else @@ -94,9 +88,6 @@ #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false #endif -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS false -#endif #if defined _LIBC || !FUNC_REALPATH_WORKS @@ -106,14 +97,11 @@ static bool file_accessible (char const *file) { # if defined _LIBC || HAVE_FACCESSAT - int r = __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); + return __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS) == 0; # else struct stat st; - int r = __stat (file, &st); + return __stat (file, &st) == 0 || errno == EOVERFLOW; # endif - - return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW) - || r == 0); } /* True if concatenating END as a suffix to a file name means that the diff --git a/lib/canonicalize.c b/lib/canonicalize.c index 48a49e412..a4d3aab96 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -46,9 +46,6 @@ #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false #endif -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS false -#endif #if ISSLASH ('\\') # define SLASHES "/\\" @@ -62,14 +59,11 @@ static bool file_accessible (char const *file) { # if HAVE_FACCESSAT - int r = faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); + return faccessat (AT_FDCWD, file, F_OK, AT_EACCESS) == 0; # else struct stat st; - int r = stat (file, &st); + return stat (file, &st) == 0 || errno == EOVERFLOW; # endif - - return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW) - || r == 0); } /* True if concatenating END as a suffix to a file name means that the diff --git a/lib/faccessat.c b/lib/faccessat.c index 330c54a0b..9f6a11bf6 100644 --- a/lib/faccessat.c +++ b/lib/faccessat.c @@ -32,13 +32,6 @@ #include <sys/stat.h> #undef _GL_INCLUDING_UNISTD_H -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS 0 -#endif -#ifndef LSTAT_FOLLOWS_SLASHED_SYMLINK -# define LSTAT_FOLLOWS_SLASHED_SYMLINK 0 -#endif - #if HAVE_FACCESSAT static int orig_faccessat (int fd, char const *name, int mode, int flag) @@ -66,12 +59,7 @@ rpl_faccessat (int fd, char const *file, int mode, int flag) { int result = orig_faccessat (fd, file, mode, flag); - if (result != 0) - { - if (!FACCESSAT_NEVER_EOVERFLOWS && mode == F_OK && errno == EOVERFLOW) - return 0; - } - else if (!LSTAT_FOLLOWS_SLASHED_SYMLINK && file[strlen (file) - 1] == '/') + if (result == 0 && file[strlen (file) - 1] == '/') { struct stat st; result = fstatat (fd, file, &st, 0); diff --git a/m4/canonicalize.m4 b/m4/canonicalize.m4 index c8da4dfcb..404db4cb6 100644 --- a/m4/canonicalize.m4 +++ b/m4/canonicalize.m4 @@ -1,4 +1,4 @@ -# canonicalize.m4 serial 34 +# canonicalize.m4 serial 35 dnl Copyright (C) 2003-2007, 2009-2020 Free Software Foundation, Inc. @@ -11,7 +11,6 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_FUNC_CANONICALIZE_FILENAME_MODE], [ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat]) AC_REQUIRE([gl_DOUBLE_SLASH_ROOT]) @@ -58,7 +57,6 @@ AC_DEFUN([gl_CANONICALIZE_LGPL], AC_DEFUN([gl_CANONICALIZE_LGPL_SEPARATE], [ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat]) diff --git a/m4/faccessat.m4 b/m4/faccessat.m4 index a4ad31a46..b8da51e4c 100644 --- a/m4/faccessat.m4 +++ b/m4/faccessat.m4 @@ -1,4 +1,4 @@ -# serial 9 +# serial 10 # See if we need to provide faccessat replacement. dnl Copyright (C) 2009-2020 Free Software Foundation, Inc. @@ -8,31 +8,6 @@ dnl with or without modifications, as long as this notice is preserved. # Written by Eric Blake. -AC_DEFUN([gl_FUNC_FACCESSAT_EOVERFLOW], -[ - AC_CHECK_FUNCS_ONCE([faccessat]) - if test "$ac_cv_func_faccessat" = yes; then - AC_CACHE_CHECK([whether faccessat works when stat would EOVERFLOW], - [gl_cv_func_faccessat_never_eoverflows], - [AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM([], - [[#ifdef __linux__ - #include <linux/version.h> - #if (! (KERNEL_VERSION (5, 8, 0) <= LINUX_VERSION_CODE \ - && 2 < (__GLIBC__ + (33 <= __GLIBC_MINOR__)))) - #error "faccessat might fail with EOVERFLOW" - #endif - #endif - ]])], - [gl_cv_func_faccessat_never_eoverflows=yes], - [gl_cv_func_faccessat_never_eoverflows=no])]) - if test "$gl_cv_func_faccessat_never_eoverflows" = yes; then - AC_DEFINE([FACCESSAT_NEVER_EOVERFLOWS], 1, - [Define to 1 if faccessat is EOVERFLOW-free.]) - fi - fi -]) - AC_DEFUN([gl_FUNC_FACCESSAT], [ AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) @@ -41,14 +16,12 @@ AC_DEFUN([gl_FUNC_FACCESSAT], dnl Persuade glibc <unistd.h> to declare faccessat(). AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) - AC_CHECK_FUNCS_ONCE([faccessat]) if test $ac_cv_func_faccessat = no; then HAVE_FACCESSAT=0 else - case $gl_cv_func_lstat_dereferences_slashed_symlink,$gl_cv_func_faccessat_never_eoverflows in - *yes,*yes) ;; + case $gl_cv_func_lstat_dereferences_slashed_symlink in + *yes) ;; *) REPLACE_FACCESSAT=1 ;; esac fi diff --git a/modules/canonicalize b/modules/canonicalize index 29919bcdc..5003f2682 100644 --- a/modules/canonicalize +++ b/modules/canonicalize @@ -5,7 +5,6 @@ Files: lib/canonicalize.h lib/canonicalize.c m4/canonicalize.m4 -m4/faccessat.m4 m4/lstat.m4 Depends-on: diff --git a/modules/canonicalize-lgpl b/modules/canonicalize-lgpl index b5f3e7f69..a96f9011e 100644 --- a/modules/canonicalize-lgpl +++ b/modules/canonicalize-lgpl @@ -5,7 +5,6 @@ Files: lib/canonicalize-lgpl.c m4/canonicalize.m4 m4/double-slash-root.m4 -m4/faccessat.m4 m4/lstat.m4 Depends-on: -- 2.27.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-01-01 3:31 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-24 15:16 [PATCH 0/5] Fix multiple realpath issues Adhemerval Zanella 2020-12-24 15:16 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella 2020-12-24 22:45 ` Paul Eggert 2020-12-25 0:44 ` [PATCH 1/5] warnings in canonicalize.c Bruno Haible 2020-12-25 5:56 ` Paul Eggert 2020-12-28 12:53 ` [PATCH 1/5] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26241] Adhemerval Zanella 2020-12-24 15:16 ` [PATCH 2/5] Import idx.h from gnulib Adhemerval Zanella 2020-12-24 22:53 ` Paul Eggert 2020-12-25 20:34 ` Florian Weimer 2020-12-25 21:38 ` Bruno Haible 2020-12-26 1:29 ` Paul Eggert 2020-12-31 23:12 ` Joseph Myers 2021-01-01 3:24 ` Paul Eggert 2020-12-24 15:16 ` [PATCH 3/5] Import filename.h " Adhemerval Zanella 2020-12-31 23:13 ` Joseph Myers 2021-01-01 3:31 ` Paul Eggert 2020-12-24 15:17 ` [PATCH 4/5] stdlib: Add testcase fro BZ #26241 Adhemerval Zanella 2020-12-24 15:17 ` [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] Adhemerval Zanella 2020-12-25 0:27 ` Paul Eggert 2020-12-28 11:42 ` Adhemerval Zanella 2020-12-28 13:32 ` Adhemerval Zanella 2020-12-28 21:04 ` Paul Eggert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).