stdlib: Simplify buffer management in canonicalize

Move the buffer management from realpath_stk to __realpath.  This
allows returning directly after allocation errors.

Always make a copy of the result buffer using strdup even if it is
already heap-allocated.  (Heap-allocated buffers are somewhat rare.)
This avoids GCC warnings at certain optimization levels.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
This commit is contained in:
Florian Weimer 2022-07-05 11:04:45 +02:00
parent 9d77023bf3
commit ef0700004b

View File

@ -49,6 +49,7 @@
#else #else
# define __canonicalize_file_name canonicalize_file_name # define __canonicalize_file_name canonicalize_file_name
# define __realpath realpath # define __realpath realpath
# define __strdup strdup
# include "pathmax.h" # include "pathmax.h"
# define __faccessat faccessat # define __faccessat faccessat
# if defined _WIN32 && !defined __CYGWIN__ # if defined _WIN32 && !defined __CYGWIN__
@ -176,27 +177,16 @@ get_path_max (void)
return path_max < 0 ? 1024 : path_max <= IDX_MAX ? path_max : IDX_MAX; return path_max < 0 ? 1024 : path_max <= IDX_MAX ? path_max : IDX_MAX;
} }
/* Act like __realpath (see below), with an additional argument /* Scratch buffers used by realpath_stk and managed by __realpath. */
rname_buf that can be used as temporary storage. struct realpath_bufs
{
struct scratch_buffer rname;
struct scratch_buffer extra;
struct scratch_buffer link;
};
If GCC_LINT is defined, do not inline this function with GCC 10.1
and later, to avoid creating a pointer to the stack that GCC
-Wreturn-local-addr incorrectly complains about. See:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644
Although the noinline attribute can hurt performance a bit, no better way
to pacify GCC is known; even an explicit #pragma does not pacify GCC.
When the GCC bug is fixed this workaround should be limited to the
broken GCC versions. */
#if __GNUC_PREREQ (10, 1)
# if defined GCC_LINT || defined lint
__attribute__ ((__noinline__))
# elif __OPTIMIZE__ && !__NO_INLINE__
# define GCC_BOGUS_WRETURN_LOCAL_ADDR
# endif
#endif
static char * static char *
realpath_stk (const char *name, char *resolved, realpath_stk (const char *name, char *resolved, struct realpath_bufs *bufs)
struct scratch_buffer *rname_buf)
{ {
char *dest; char *dest;
char const *start; char const *start;
@ -221,12 +211,7 @@ realpath_stk (const char *name, char *resolved,
return NULL; return NULL;
} }
struct scratch_buffer extra_buffer, link_buffer; char *rname = bufs->rname.data;
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 end_in_extra_buffer = false;
bool failed = true; bool failed = true;
@ -236,16 +221,16 @@ realpath_stk (const char *name, char *resolved,
if (!IS_ABSOLUTE_FILE_NAME (name)) if (!IS_ABSOLUTE_FILE_NAME (name))
{ {
while (!__getcwd (rname, rname_buf->length)) while (!__getcwd (bufs->rname.data, bufs->rname.length))
{ {
if (errno != ERANGE) if (errno != ERANGE)
{ {
dest = rname; dest = rname;
goto error; goto error;
} }
if (!scratch_buffer_grow (rname_buf)) if (!scratch_buffer_grow (&bufs->rname))
goto error_nomem; return NULL;
rname = rname_buf->data; rname = bufs->rname.data;
} }
dest = __rawmemchr (rname, '\0'); dest = __rawmemchr (rname, '\0');
start = name; start = name;
@ -299,13 +284,13 @@ realpath_stk (const char *name, char *resolved,
if (!ISSLASH (dest[-1])) if (!ISSLASH (dest[-1]))
*dest++ = '/'; *dest++ = '/';
while (rname + rname_buf->length - dest while (rname + bufs->rname.length - dest
< startlen + sizeof dir_suffix) < startlen + sizeof dir_suffix)
{ {
idx_t dest_offset = dest - rname; idx_t dest_offset = dest - rname;
if (!scratch_buffer_grow_preserve (rname_buf)) if (!scratch_buffer_grow_preserve (&bufs->rname))
goto error_nomem; return NULL;
rname = rname_buf->data; rname = bufs->rname.data;
dest = rname + dest_offset; dest = rname + dest_offset;
} }
@ -316,13 +301,13 @@ realpath_stk (const char *name, char *resolved,
ssize_t n; ssize_t n;
while (true) while (true)
{ {
buf = link_buffer.data; buf = bufs->link.data;
idx_t bufsize = link_buffer.length; idx_t bufsize = bufs->link.length;
n = __readlink (rname, buf, bufsize - 1); n = __readlink (rname, buf, bufsize - 1);
if (n < bufsize - 1) if (n < bufsize - 1)
break; break;
if (!scratch_buffer_grow (&link_buffer)) if (!scratch_buffer_grow (&bufs->link))
goto error_nomem; return NULL;
} }
if (0 <= n) if (0 <= n)
{ {
@ -334,7 +319,7 @@ realpath_stk (const char *name, char *resolved,
buf[n] = '\0'; buf[n] = '\0';
char *extra_buf = extra_buffer.data; char *extra_buf = bufs->extra.data;
idx_t end_idx IF_LINT (= 0); idx_t end_idx IF_LINT (= 0);
if (end_in_extra_buffer) if (end_in_extra_buffer)
end_idx = end - extra_buf; end_idx = end - extra_buf;
@ -342,13 +327,13 @@ realpath_stk (const char *name, char *resolved,
if (INT_ADD_OVERFLOW (len, n)) if (INT_ADD_OVERFLOW (len, n))
{ {
__set_errno (ENOMEM); __set_errno (ENOMEM);
goto error_nomem; return NULL;
} }
while (extra_buffer.length <= len + n) while (bufs->extra.length <= len + n)
{ {
if (!scratch_buffer_grow_preserve (&extra_buffer)) if (!scratch_buffer_grow_preserve (&bufs->extra))
goto error_nomem; return NULL;
extra_buf = extra_buffer.data; extra_buf = bufs->extra.data;
} }
if (end_in_extra_buffer) if (end_in_extra_buffer)
end = extra_buf + end_idx; end = extra_buf + end_idx;
@ -406,25 +391,24 @@ error:
to the path not existing or not being accessible. */ to the path not existing or not being accessible. */
if ((!failed || errno == ENOENT || errno == EACCES) if ((!failed || errno == ENOENT || errno == EACCES)
&& dest - rname <= get_path_max ()) && dest - rname <= get_path_max ())
rname = strcpy (resolved, rname);
else if (!failed)
{ {
failed = true; strcpy (resolved, rname);
__set_errno (ENAMETOOLONG); if (failed)
return NULL;
else
return resolved;
} }
if (!failed)
__set_errno (ENAMETOOLONG);
return NULL;
} }
else
error_nomem:
scratch_buffer_free (&extra_buffer);
scratch_buffer_free (&link_buffer);
if (failed || rname == resolved)
{ {
scratch_buffer_free (rname_buf); if (failed)
return failed ? NULL : resolved; return NULL;
else
return __strdup (bufs->rname.data);
} }
return scratch_buffer_dupfree (rname_buf, dest - rname);
} }
/* Return the canonical absolute name of file NAME. A canonical name /* Return the canonical absolute name of file NAME. A canonical name
@ -441,12 +425,15 @@ error_nomem:
char * char *
__realpath (const char *name, char *resolved) __realpath (const char *name, char *resolved)
{ {
#ifdef GCC_BOGUS_WRETURN_LOCAL_ADDR struct realpath_bufs bufs;
#warning "GCC might issue a bogus -Wreturn-local-addr warning here." scratch_buffer_init (&bufs.rname);
#warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>." scratch_buffer_init (&bufs.extra);
#endif scratch_buffer_init (&bufs.link);
struct scratch_buffer rname_buffer; char *result = realpath_stk (name, resolved, &bufs);
return realpath_stk (name, resolved, &rname_buffer); scratch_buffer_free (&bufs.link);
scratch_buffer_free (&bufs.extra);
scratch_buffer_free (&bufs.rname);
return result;
} }
libc_hidden_def (__realpath) libc_hidden_def (__realpath)
versioned_symbol (libc, __realpath, realpath, GLIBC_2_3); versioned_symbol (libc, __realpath, realpath, GLIBC_2_3);