Paul Eggert wrote:
> What's wrong with avoiding the problem via GCC_LINT?
Some people, especially those that build distros, want to get warnings about
the exact code that they ship in the distro, not about some different code.
> I suppose you could add -Wno-return-local-addr to your CFLAGS, but I don't
> recommend this as it's normally a useful warning.
Right.
How about this modification? It splits the function into two functions, and
unless the first function is inlined, the warning disappears.
- No use of GCC_LINT.
- The runtime cost is smaller: It still uses a stack-allocated buffer.
Just an additional function call.
diff --git a/lib/careadlinkat.c b/lib/careadlinkat.c
index 1aa0436..67ea7d7 100644
--- a/lib/careadlinkat.c
+++ b/lib/careadlinkat.c
@@ -60,45 +60,24 @@
If successful, return the buffer address; otherwise return NULL and
set errno. */
-char *
-careadlinkat (int fd, char const *filename,
- char *buffer, size_t buffer_size,
- struct allocator const *alloc,
- ssize_t (*preadlinkat) (int, char const *, char *, size_t))
+/* A broken gcc -Wreturn-local-addr would cry wolf about returning the address
+ of a stack-allocated buffer, if this function gets inlined. See:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95044
+ When the GCC bug is fixed this workaround should be limited to the broken
+ GCC versions. */
+#if _GL_GNUC_PREREQ (10, 1) && ! defined __clang__
+__attribute__ ((__noinline__))
+#endif
+static char *
+careadlinkat_internal (int fd, char const *filename,
+ char *buffer, size_t buffer_size,
+ struct allocator const *alloc,
+ ssize_t (*preadlinkat) (int, char const *, char *,
size_t))
{
char *buf;
size_t buf_size;
size_t buf_size_max =
SSIZE_MAX < SIZE_MAX ? (size_t) SSIZE_MAX + 1 : SIZE_MAX;
- char stack_buf[1024];
-
-#if (defined GCC_LINT || defined lint) && _GL_GNUC_PREREQ (10, 1)
- /* Pacify preadlinkat without creating a pointer to the stack
- that a broken gcc -Wreturn-local-addr would cry wolf about. See:
- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95044
- This workaround differs from the mainline code, but
- no other way to pacify GCC 10.1.0 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. */
-# define WORK_AROUND_GCC_BUG_95044
-#endif
-
- if (! alloc)
- alloc = &stdlib_allocator;
-
- if (!buffer)
- {
-#ifdef WORK_AROUND_GCC_BUG_95044
- buffer = alloc->allocate (sizeof stack_buf);
-#else
- /* Allocate the initial buffer on the stack. This way, in the
- common case of a symlink of small size, we get away with a
- single small malloc() instead of a big malloc() followed by a
- shrinking realloc(). */
- buffer = stack_buf;
-#endif
- buffer_size = sizeof stack_buf;
- }
buf = buffer;
buf_size = buffer_size;
@@ -130,15 +109,6 @@ careadlinkat (int fd, char const *filename,
{
buf[link_size++] = '\0';
- if (buf == stack_buf)
- {
- char *b = alloc->allocate (link_size);
- buf_size = link_size;
- if (! b)
- break;
- return memcpy (b, buf, link_size);
- }
-
if (link_size < buf_size && buf != buffer && alloc->reallocate)
{
/* Shrink BUF before returning it. */
@@ -172,3 +142,44 @@ careadlinkat (int fd, char const *filename,
errno = ENOMEM;
return NULL;
}
+
+char *
+careadlinkat (int fd, char const *filename,
+ char *buffer, size_t buffer_size,
+ struct allocator const *alloc,
+ ssize_t (*preadlinkat) (int, char const *, char *, size_t))
+{
+ char stack_buf[1024];
+
+ if (! alloc)
+ alloc = &stdlib_allocator;
+
+ if (!buffer)
+ {
+ /* Allocate the initial buffer on the stack. This way, in the
+ common case of a symlink of small size, we get away with a
+ single small malloc() instead of a big malloc() followed by a
+ shrinking realloc(). */
+ buffer = stack_buf;
+ buffer_size = sizeof stack_buf;
+ }
+
+ char *buf = careadlinkat_internal (fd, filename, buffer, buffer_size, alloc,
+ preadlinkat);
+
+ if (buf == stack_buf)
+ {
+ size_t link_size = strlen (buf) + 1;
+ char *b = alloc->allocate (link_size);
+ if (b)
+ return memcpy (b, buf, link_size);
+ else
+ {
+ if (alloc->die)
+ alloc->die (link_size);
+ errno = ENOMEM;
+ return NULL;
+ }
+ }
+ return buf;
+}
