Re: incorrect use of 'pure' attribute
Paul Eggert <[email protected]>
| Newsgroups | gmane.comp.lib.gnulib.bugs |
|---|---|
| Organization | UCLA Computer Science Department |
| Message-ID | <[email protected]> |
On 2026-08-09 07:11, Bruno Haible wrote: > The scenario you describe can only occur on Hurd/m68k I thought so too, but more recently I discovered that the sysconf call can also return -1 on GNU/Linux, OpenBSD, Solaris, and probably other OSes. GNU/Linux still runs on the Motorola 68000 and 68010 architectures if you configure Linux with CONFIG_MMU=n. It even runs on the 68008![1] So the ice is getting a little thin here. But we're in luck: last year glibc removed the __eloop_threshold function, presumably because there's not much point to it: nobody uses symlink loop counts greater than 40 in practice, so a compile-time limit of 40 conforms to POSIX and is simpler to maintain. So I fixed the 'const' attribute issue by following glibc's lead and removing the problematic __eloop_threshold function, resulting in the attached patch which I installed into Gnulib. [1]: https://hackaday.io/project/183861-mackerel-68k-linux-sbcs/log/248289-mainline-linux-71-on-a-motorola-68008-the-slowest-still-supported-cpu
0001-canonicalize-lgpl-sync-from-glibc.patch
(text/x-patch, 15.2 KB)
From bb4e43616575681417f7bae48792651fc6ce489f Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Mon, 10 Aug 2026 13:08:08 -0700 Subject: [PATCH] canonicalize-lgpl: sync from glibc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge into lib/canonicalize-lgpl.c the changes of glibc stdlib/canonicalize.c through glibc commit b589bd672c529cf264dc6dfdfa11f73c7e4e1666 dated today. The result should be copiable back to glibc byte-for-byte. The original motivation for this merge was to make eloop-threshold.h thread-safe on all platforms. It turned out that glibc has already done this, trivially, by having the include file define a constant 40 rather than a function. This conforms to POSIX on all glibc targets, and it should also work on all Gnulib targets that I know of, as sysconf (_SC_SYMLOOP_MAX) <= 40 on all known platforms. POSIX allows us to follow more symlink expansions than any nonnegative number returned by sysconf (_SC_SYMLOOP_MAX), and if that number is negative POSIX requires only to expand at least 8 symlinks so the glibc value of 40 is good then too. * NEWS: Mention the incompatible change. * config/srclist.txt: Add lib/min-eloop-threshold.h. * lib/canonicalize-lgpl.c [_LIBC]: Include <stdbool.h>. (__rawmemchr): Remove. Replace its use with strchr. * lib/min-eloop-threshold.h: Sync byte-for-byte from gnulibc, renaming the file from lib/eloop-threshold.h. All uses changed. This entails the following changes: (_MIN_ELOOP_THRESHOLD_H): Rename from _ELOOP_THRESHOLD_H. All uses changed. Don’t #error out if config.h is not already included. Don’t include <limits.h>. [_LIBC]: Don’t include <sys/param.h>. (_GL_ATTRIBUTE_CONST) [_LIBC]: Remove. [!_LIBC]: Don’t include <unistd.h>, "minmax.h". (__sysconf, SYMLOOP_MAX) [!_LIBC]: Remove. (MIN_ELOOP_THRESHOLD): Define unconditionally. (__eloop_threshold): Remove. All uses replaced by MIN_ELOOP_THRESHOLD. The new expression is of type int not unsigned, but that’s better for callers anyway. * lib/openat2.c (do_openat2): Symlink count is now int, not long int. * modules/eloop-threshold (Files, Include): Adjust to file name change. (Depends-on): Remove minmax, unistd-h. --- ChangeLog | 41 ++++++++++++++++++ NEWS | 4 ++ config/srclist.txt | 2 +- lib/canonicalize-lgpl.c | 8 ++-- lib/eloop-threshold.h | 88 --------------------------------------- lib/linkat.c | 6 +-- lib/min-eloop-threshold.h | 42 +++++++++++++++++++ lib/openat2.c | 4 +- modules/eloop-threshold | 6 +-- 9 files changed, 99 insertions(+), 102 deletions(-) delete mode 100644 lib/eloop-threshold.h create mode 100644 lib/min-eloop-threshold.h diff --git a/ChangeLog b/ChangeLog index fade982eb5..e258e18d2b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,44 @@ +2026-08-10 Paul Eggert <[email protected]> + + canonicalize-lgpl: sync from glibc + Merge into lib/canonicalize-lgpl.c the changes of glibc + stdlib/canonicalize.c through glibc commit + b589bd672c529cf264dc6dfdfa11f73c7e4e1666 dated today. + The result should be copiable back to glibc byte-for-byte. + The original motivation for this merge was to make + eloop-threshold.h thread-safe on all platforms. + It turned out that glibc has already done this, trivially, + by having the include file define a constant 40 rather than a function. + This conforms to POSIX on all glibc targets, and it should also + work on all Gnulib targets that I know of, + as sysconf (_SC_SYMLOOP_MAX) <= 40 on all known platforms. + POSIX allows us to follow more symlink expansions than + any nonnegative number returned by sysconf (_SC_SYMLOOP_MAX), + and if that number is negative POSIX requires only to expand + at least 8 symlinks so the glibc value of 40 is good then too. + * NEWS: Mention the incompatible change. + * config/srclist.txt: Add lib/min-eloop-threshold.h. + * lib/canonicalize-lgpl.c [_LIBC]: Include <stdbool.h>. + (__rawmemchr): Remove. Replace its use with strchr. + * lib/min-eloop-threshold.h: Sync byte-for-byte from gnulibc, + renaming the file from lib/eloop-threshold.h. All uses changed. + This entails the following changes: + (_MIN_ELOOP_THRESHOLD_H): Rename from _ELOOP_THRESHOLD_H. + All uses changed. + Don’t #error out if config.h is not already included. + Don’t include <limits.h>. + [_LIBC]: Don’t include <sys/param.h>. + (_GL_ATTRIBUTE_CONST) [_LIBC]: Remove. + [!_LIBC]: Don’t include <unistd.h>, "minmax.h". + (__sysconf, SYMLOOP_MAX) [!_LIBC]: Remove. + (MIN_ELOOP_THRESHOLD): Define unconditionally. + (__eloop_threshold): Remove. All uses replaced by MIN_ELOOP_THRESHOLD. + The new expression is of type int not unsigned, but that’s better + for callers anyway. + * lib/openat2.c (do_openat2): Symlink count is now int, not long int. + * modules/eloop-threshold (Files, Include): Adjust to file name change. + (Depends-on): Remove minmax, unistd-h. + 2026-08-09 Paul Eggert <[email protected]> hamt: port to non-_Atomic C platforms diff --git a/NEWS b/NEWS index db7776ee78..28a5869ee8 100644 --- a/NEWS +++ b/NEWS @@ -78,6 +78,10 @@ User visible incompatible changes Date Modules Changes +2026-08-10 eloop-threshold Now include <min-eloop-threshold.h> and use + MIN_ELOOP_THRESHOLD rather than including + <eloop-threshold.h> and using __eloop_threshold (). + 2026-06-26 bitrotate This module is deprecated. Use stdc_rotate_left or stdc_rotate_right instead. diff --git a/config/srclist.txt b/config/srclist.txt index c9e6b4336e..a19a8b78ba 100644 --- a/config/srclist.txt +++ b/config/srclist.txt @@ -89,7 +89,7 @@ $LIBCSRC stdlib/tst-stdc_leading_ones.c tests/from-glibc $LIBCSRC stdlib/tst-stdc_leading_zeros.c tests/from-glibc $LIBCSRC stdlib/tst-stdc_trailing_ones.c tests/from-glibc $LIBCSRC stdlib/tst-stdc_trailing_zeros.c tests/from-glibc -#$LIBCSRC sysdeps/generic/eloop-threshold.h lib +$LIBCSRC sysdeps/generic/min-eloop-threshold.h lib #$LIBCSRC time/timegm.c lib #$LIBCSRC time/mktime.c lib #$LIBCSRC time/mktime-internal.h lib diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index a87d46a433..92c96243c2 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -34,7 +34,7 @@ #include <sys/stat.h> #include <unistd.h> -#include <eloop-threshold.h> +#include <min-eloop-threshold.h> #include <filename.h> #include <idx.h> #include <intprops.h> @@ -42,6 +42,7 @@ #ifdef _LIBC # include <shlib-compat.h> +# include <stdbool.h> # define GCC_LINT 1 # define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__)) #else @@ -72,7 +73,6 @@ # endif # define __mempcpy mempcpy # define __pathconf pathconf -# define __rawmemchr rawmemchr # define __readlink readlink # if IN_RELOCWRAPPER /* When building the relocatable program wrapper, use the system's memmove @@ -233,7 +233,7 @@ realpath_stk (const char *name, char *resolved, struct realpath_bufs *bufs) return NULL; rname = bufs->rname.data; } - dest = __rawmemchr (rname, '\0'); + dest = strchr (rname, '\0'); start = name; prefix_len = FILE_SYSTEM_PREFIX_LEN (rname); } @@ -317,7 +317,7 @@ realpath_stk (const char *name, char *resolved, struct realpath_bufs *bufs) } if (0 <= n) { - if (++num_links > __eloop_threshold ()) + if (++num_links > MIN_ELOOP_THRESHOLD) { __set_errno (ELOOP); goto error; diff --git a/lib/eloop-threshold.h b/lib/eloop-threshold.h deleted file mode 100644 index 3e33a24708..0000000000 --- a/lib/eloop-threshold.h +++ /dev/null @@ -1,88 +0,0 @@ -/* Threshold at which to diagnose ELOOP. Generic version. - Copyright (C) 2012-2026 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 _ELOOP_THRESHOLD_H -#define _ELOOP_THRESHOLD_H 1 - -/* This file uses _GL_ATTRIBUTE_CONST. */ -#if !_LIBC && !_GL_CONFIG_H_INCLUDED - #error "Please include config.h first." -#endif - -#include <limits.h> -#ifdef _LIBC -# include <sys/param.h> -# define _GL_ATTRIBUTE_CONST __attribute__ ((const)) -#else -# include <unistd.h> -# include "minmax.h" -# define __sysconf sysconf -# if (!defined SYMLOOP_MAX \ - && ! (defined _SC_SYMLOOP_MAX && defined _POSIX_SYMLOOP_MAX)) -# define SYMLOOP_MAX 8 -# endif -#endif - -/* POSIX specifies SYMLOOP_MAX as the "Maximum number of symbolic - links that can be reliably traversed in the resolution of a - pathname in the absence of a loop." This makes it a minimum that - we should certainly accept. But it leaves open the possibility - that more might sometimes work--just not "reliably". - - For example, Linux implements a complex policy whereby there is a - small limit on the number of direct symlink traversals (a symlink - to a symlink to a symlink), but larger limit on the total number of - symlink traversals overall. Hence the SYMLOOP_MAX number should be - the small one, but the limit library functions enforce on users - should be the larger one. - - So, we use the larger of the reported SYMLOOP_MAX (if any) and our - own constant MIN_ELOOP_THRESHOLD, below. This constant should be - large enough that it never rules out a file name and directory tree - that the underlying system (i.e. calls to 'open' et al) would - resolve successfully. It should be small enough that actual loops - are detected without a huge number of iterations. */ - -#ifndef MIN_ELOOP_THRESHOLD -# define MIN_ELOOP_THRESHOLD 40 -#endif - -/* Return the maximum number of symlink traversals to permit - before diagnosing ELOOP. */ -static inline unsigned int _GL_ATTRIBUTE_CONST -__eloop_threshold (void) -{ -#ifdef SYMLOOP_MAX - const int symloop_max = SYMLOOP_MAX; -#else - /* The function is marked 'const' even though we use memory and - call a function, because sysconf is required to return the - same value in every call and so it must always be safe to - call __eloop_threshold exactly once and reuse the value. */ - static long int sysconf_symloop_max; - if (sysconf_symloop_max == 0) - sysconf_symloop_max = __sysconf (_SC_SYMLOOP_MAX); - const unsigned int symloop_max = (sysconf_symloop_max <= 0 - ? _POSIX_SYMLOOP_MAX - : sysconf_symloop_max); -#endif - - return MAX (symloop_max, MIN_ELOOP_THRESHOLD); -} - -#endif /* eloop-threshold.h */ diff --git a/lib/linkat.c b/lib/linkat.c index 0225e41d4a..8fc105fe99 100644 --- a/lib/linkat.c +++ b/lib/linkat.c @@ -29,8 +29,8 @@ #include "areadlink.h" #include "dirname.h" -#include "eloop-threshold.h" #include "filenamecat.h" +#include "min-eloop-threshold.h" #include "openat-priv.h" #if !HAVE_LINKAT || LINKAT_SYMLINK_NOTSUP @@ -92,7 +92,7 @@ link_follow (char const *file1, char const *file2) { char *name = (char *) file1; char *target; - int i = __eloop_threshold (); + int i = MIN_ELOOP_THRESHOLD; /* Using realpath or canonicalize_file_name is too heavy-handed: we don't need an absolute name, and we don't need to resolve @@ -209,7 +209,7 @@ linkat_follow (int fd1, char const *file1, int fd2, char const *file2) { char *name = (char *) file1; char *target; - int i = __eloop_threshold (); + int i = MIN_ELOOP_THRESHOLD; /* There is no realpathat. */ while (i-- && (target = areadlinkat (fd1, name))) diff --git a/lib/min-eloop-threshold.h b/lib/min-eloop-threshold.h new file mode 100644 index 0000000000..26b0795a64 --- /dev/null +++ b/lib/min-eloop-threshold.h @@ -0,0 +1,42 @@ +/* Minimum threshold at which to diagnose ELOOP. Generic version. + Copyright (C) 2012-2026 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 _MIN_ELOOP_THRESHOLD_H +#define _MIN_ELOOP_THRESHOLD_H 1 + +/* POSIX specifies SYMLOOP_MAX as the "Maximum number of symbolic + links that can be reliably traversed in the resolution of a + pathname in the absence of a loop." This makes it a minimum that + we should certainly accept. But it leaves open the possibility + that more might sometimes work--just not "reliably". + + For example, Linux implements a complex policy whereby there is a + small limit on the number of direct symlink traversals (a symlink + to a symlink to a symlink), but larger limit on the total number of + symlink traversals overall. Hence the SYMLOOP_MAX number should be + the small one, but the limit library functions enforce on users + should be the larger one. + + This constant should be large enough that it never rules out a file + name and directory tree that the underlying system (i.e. calls to 'open' + et al) would resolve successfully. It should be small enough that + actual loops are detected without a huge number of iterations. */ + +#define MIN_ELOOP_THRESHOLD 40 + +#endif diff --git a/lib/openat2.c b/lib/openat2.c index ff8ce30263..e860252654 100644 --- a/lib/openat2.c +++ b/lib/openat2.c @@ -20,10 +20,10 @@ #include <fcntl.h> -#include "eloop-threshold.h" #include "filename.h" #include "ialloc.h" #include "idx.h" +#include "min-eloop-threshold.h" #include "verify.h" #include <errno.h> @@ -228,7 +228,7 @@ do_openat2 (int *fd, char const *filename, dev_t const UNKNOWN_DEV = -1; dev_t ddev = UNKNOWN_DEV; - long int maxlinks = resolve & RESOLVE_NO_SYMLINKS ? 0 : __eloop_threshold (); + int maxlinks = resolve & RESOLVE_NO_SYMLINKS ? 0 : MIN_ELOOP_THRESHOLD; int dfd = *fd; diff --git a/modules/eloop-threshold b/modules/eloop-threshold index 3cb5834cc6..9a95eb54ad 100644 --- a/modules/eloop-threshold +++ b/modules/eloop-threshold @@ -2,18 +2,16 @@ Description: Threshold for symbolic link loops and ELOOP Files: -lib/eloop-threshold.h +lib/min-eloop-threshold.h Depends-on: -minmax -unistd-h configure.ac: Makefile.am: Include: -<eloop-threshold.h> +<min-eloop-threshold.h> License: LGPLv2+ -- 2.55.0