[PATCH 3/3] mktime: improve thread-safety
* lib/mktime.c (__mktime_internal) [!_LIBC]: Double the number of probes. Although this isn’t guaranteed to suffice, it should be good enough for practical applications, and fixing the problem in general would require access to the underlying tz state lock which would be hard to do. --- ChangeLog| 7 +++ lib/mktime.c | 8 2 files changed, 15 insertions(+) diff --git a/ChangeLog b/ChangeLog index cea31f84e6..65fd11b2bf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2024-10-04 Paul Eggert + mktime: improve thread-safety + * lib/mktime.c (__mktime_internal) [!_LIBC]: Double the number of + probes. Although this isn’t guaranteed to suffice, it should be + good enough for practical applications, and fixing the problem + in general would require access to the underlying tz state lock + which would be hard to do. + mktime: fix timegm bug that set tmp->tm_isdst * lib/timegm.c (__timegm64): Omit now-unnecessary initialization of tm_isdst. Anyway, the initialization was always wrong, since diff --git a/lib/mktime.c b/lib/mktime.c index f21dfe3838..7f07abb814 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -332,6 +332,14 @@ __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) leap seconds, but some hosts have them anyway. */ int remaining_probes = 6; +#ifndef _LIBC + /* Gnulib mktime doesn't lock the tz state, so it may need to probe + more often if some other thread changes local time while + __mktime_internal is probing. Double the number of probes; this + should suffice for practical cases that are at all likely. */ + remaining_probes *= 2; +#endif + /* Time requested. Copy it in case gmtime/localtime modify *TP; this can occur if TP is localtime's returned value and CONVERT is localtime. */ -- 2.43.0
[PATCH 1/3] mktime: refactor to get closer to glibc
* lib/mktime.c (convert_time): Reorder args. (__tz_convert): New macro. All convert_time callers changed to use it. --- ChangeLog| 6 ++ lib/mktime.c | 14 -- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index e5d5b10d37..6d8a1a52b3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2024-10-04 Paul Eggert + + mktime: refactor to get closer to glibc + * lib/mktime.c (convert_time): Reorder args. + (__tz_convert): New macro. All convert_time callers changed to use it. + 2024-10-04 Bruno Haible iconv_open: Fix undefined behaviour. diff --git a/lib/mktime.c b/lib/mktime.c index 67bfcb956f..f21dfe3838 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -254,7 +254,7 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec, otherwise gmtime64_r. T must be in range for __time64_t. Return TM if successful, NULL (setting errno) on failure. */ static struct tm * -convert_time (bool local, long_int t, struct tm *tm) +convert_time (long_int t, bool local, struct tm *tm) { __time64_t x = t; if (local) @@ -262,6 +262,8 @@ convert_time (bool local, long_int t, struct tm *tm) else return __gmtime64_r (&x, tm); } +/* Call it __tzconvert to sync with other parts of glibc. */ +#define __tz_convert convert_time /* Convert *T to a broken down time in *TP (as if by localtime if LOCAL, otherwise as if by gmtime). If *T is out of range for @@ -274,7 +276,7 @@ ranged_convert (bool local, long_int *t, struct tm *tp) { long_int t1 = (*t < mktime_min ? mktime_min : *t <= mktime_max ? *t : mktime_max); - struct tm *r = convert_time (local, t1, tp); + struct tm *r = __tz_convert (t1, local, tp); if (r) { *t = t1; @@ -295,7 +297,7 @@ ranged_convert (bool local, long_int *t, struct tm *tp) long_int mid = long_int_avg (ok, bad); if (mid == ok || mid == bad) break; - if (convert_time (local, mid, tp)) + if (__tz_convert (mid, local, tp)) ok = mid, oktm = *tp; else if (errno != EOVERFLOW) return NULL; @@ -481,7 +483,7 @@ __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) &otm); if (mktime_min <= gt && gt <= mktime_max) { - if (convert_time (local, gt, &tm)) + if (__tz_convert (gt, local, &tm)) { t = gt; goto offset_found; @@ -495,7 +497,7 @@ __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) /* No unusual DST offset was found nearby. Assume one-hour DST. */ t += 60 * 60 * dst_difference; - if (mktime_min <= t && t <= mktime_max && convert_time (local, t, &tm)) + if (mktime_min <= t && t <= mktime_max && __tz_convert (t, local, &tm)) goto offset_found; __set_errno (EOVERFLOW); @@ -522,7 +524,7 @@ __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) __set_errno (EOVERFLOW); return -1; } - if (! convert_time (local, t, &tm)) + if (! __tz_convert (t, local, &tm)) return -1; } -- 2.43.0
[PATCH 2/3] mktime: fix timegm bug that set tmp->tm_isdst
* lib/timegm.c (__timegm64): Omit now-unnecessary initialization of tm_isdst. Anyway, the initialization was always wrong, since timegm should not modify *TMP when it fails. --- ChangeLog| 5 + lib/timegm.c | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 6d8a1a52b3..cea31f84e6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2024-10-04 Paul Eggert + mktime: fix timegm bug that set tmp->tm_isdst + * lib/timegm.c (__timegm64): Omit now-unnecessary initialization + of tm_isdst. Anyway, the initialization was always wrong, since + timegm should not modify *TMP when it fails. + mktime: refactor to get closer to glibc * lib/mktime.c (convert_time): Reorder args. (__tz_convert): New macro. All convert_time callers changed to use it. diff --git a/lib/timegm.c b/lib/timegm.c index 1f1f66c61b..ba28b3ecd9 100644 --- a/lib/timegm.c +++ b/lib/timegm.c @@ -30,7 +30,6 @@ __time64_t __timegm64 (struct tm *tmp) { static mktime_offset_t gmtime_offset; - tmp->tm_isdst = 0; return __mktime_internal (tmp, false, &gmtime_offset); } -- 2.43.0
[PATCH 2/2] timegm: ignore incoming tm_isdst
Problem reported by Florian Weimer via a proposed glibc patch in: https://sourceware.org/pipermail/libc-alpha/2024-October/160310.html * lib/mktime.c (__mktime_internal): Ignore any tm_isdst request if the timezone never observes DST, as is the case for timegm. * m4/mktime.m4 (gl_PREREQ_MKTIME): Define new C macro __daylight if needed. --- ChangeLog| 8 lib/mktime.c | 4 +++- m4/mktime.m4 | 22 -- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index d8ec6067da..7a7f6dd782 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2024-10-04 Paul Eggert + timegm: ignore incoming tm_isdst + Problem reported by Florian Weimer via a proposed glibc patch in: + https://sourceware.org/pipermail/libc-alpha/2024-October/160310.html + * lib/mktime.c (__mktime_internal): Ignore any tm_isdst request + if the timezone never observes DST, as is the case for timegm. + * m4/mktime.m4 (gl_PREREQ_MKTIME): Define new C macro __daylight + if needed. + timegm: desync from glibc for now * config/srclist.txt: Omit time/timegm.c and time/mktime-internal.h for now, until we can sync glibc from Gnulib. diff --git a/lib/mktime.c b/lib/mktime.c index 561c948713..67bfcb956f 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -339,7 +339,9 @@ __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) int mday = tp->tm_mday; int mon = tp->tm_mon; int year_requested = tp->tm_year; - int isdst = tp->tm_isdst; + + /* If the timezone never observes DST, ignore any tm_isdst request. */ + int isdst = local && __daylight ? tp->tm_isdst : 0; /* 1 if the previous probe was DST. */ int dst2 = 0; diff --git a/m4/mktime.m4 b/m4/mktime.m4 index 85c52454aa..14ced571e0 100644 --- a/m4/mktime.m4 +++ b/m4/mktime.m4 @@ -1,5 +1,5 @@ # mktime.m4 -# serial 39 +# serial 40 dnl Copyright (C) 2002-2003, 2005-2007, 2009-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -315,4 +315,22 @@ AC_DEFUN([gl_FUNC_MKTIME_INTERNAL], [ ]) # Prerequisites of lib/mktime.c. -AC_DEFUN([gl_PREREQ_MKTIME], [:]) +AC_DEFUN([gl_PREREQ_MKTIME], [ + AC_CACHE_CHECK([spelling of daylight variable], +[gl_cv_var___daylight], +[for gl_cv_var___daylight in __daylight daylight _daylight 0; do + test $gl_cv_var___daylight = 0 && break + AC_LINK_IFELSE( + [AC_LANG_PROGRAM( +[[#include +]], +[[return $gl_cv_var___daylight;]]) + ], + [break]) + done]) + AS_CASE([$gl_cv_var___daylight], +[__daylight], [], +[AC_DEFINE_UNQUOTED([__daylight], [$gl_cv_var___daylight], + [Define to an expression equivalent to daylight +if __daylight does not already do that.])]) +]) -- 2.43.0
[PATCH 1/2] timegm: desync from glibc for now
* config/srclist.txt: Omit time/timegm.c and time/mktime-internal.h for now, until we can sync glibc from Gnulib. * lib/mktime-internal.h, lib/timegm.c: Revert autoupdate, going back to the recent Gnulib-specific version. --- ChangeLog | 8 config/srclist.txt| 4 ++-- lib/mktime-internal.h | 9 - lib/timegm.c | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4316c1bed4..d8ec6067da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2024-10-04 Paul Eggert + + timegm: desync from glibc for now + * config/srclist.txt: Omit time/timegm.c and time/mktime-internal.h + for now, until we can sync glibc from Gnulib. + * lib/mktime-internal.h, lib/timegm.c: Revert autoupdate, going + back to the recent Gnulib-specific version. + 2024-10-03 Paul Eggert mktime: prefer static_assert to verify diff --git a/config/srclist.txt b/config/srclist.txt index dbf32d5246..173f23edaf 100644 --- a/config/srclist.txt +++ b/config/srclist.txt @@ -89,9 +89,9 @@ $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.hlib -$LIBCSRC time/timegm.c lib +#$LIBCSRC time/timegm.clib #$LIBCSRC time/mktime.clib -$LIBCSRC time/mktime-internal.hlib +#$LIBCSRC time/mktime-internal.h lib # # All below here commented out in forlorn hope of future syncs. diff --git a/lib/mktime-internal.h b/lib/mktime-internal.h index 0693aaf140..3e2848c121 100644 --- a/lib/mktime-internal.h +++ b/lib/mktime-internal.h @@ -71,9 +71,8 @@ typedef int mktime_offset_t; #endif /* Subroutine of mktime. Return the time_t representation of TP and - normalize TP, given that a struct tm * maps to a time_t as performed - by FUNC. Record next guess for localtime-gmtime offset in *OFFSET. */ -extern __time64_t __mktime_internal (struct tm *tp, - struct tm *(*func) (__time64_t const *, - struct tm *), + normalize TP, given that a struct tm * maps to a time_t. If + LOCAL, the mapping is performed by localtime_r, otherwise by gmtime_r. + Record next guess for localtime-gmtime offset in *OFFSET. */ +extern __time64_t __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) attribute_hidden; diff --git a/lib/timegm.c b/lib/timegm.c index e5cf30c019..1f1f66c61b 100644 --- a/lib/timegm.c +++ b/lib/timegm.c @@ -31,7 +31,7 @@ __timegm64 (struct tm *tmp) { static mktime_offset_t gmtime_offset; tmp->tm_isdst = 0; - return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset); + return __mktime_internal (tmp, false, &gmtime_offset); } #if defined _LIBC && __TIMESIZE != 64 -- 2.43.0
[PATCH 4/4] mktime: prefer static_assert to verify
This should work better with glibc. * lib/mktime.c: Do not include verify.h. Use static_assert instead of verify. * modules/mktime (Depends-on): Depend on assert-h, not verify. --- ChangeLog | 6 ++ lib/mktime.c | 7 +++ modules/mktime | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 08a452130d..4316c1bed4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2024-10-03 Paul Eggert + mktime: prefer static_assert to verify + This should work better with glibc. + * lib/mktime.c: Do not include verify.h. + Use static_assert instead of verify. + * modules/mktime (Depends-on): Depend on assert-h, not verify. + mktime: refactor convert_time This is to better merge with a future version of glibc. This merges a glibc patch by Florian Weimer in: diff --git a/lib/mktime.c b/lib/mktime.c index a4974de63f..561c948713 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -51,7 +51,6 @@ #include #include -#include #ifndef NEED_MKTIME_INTERNAL # define NEED_MKTIME_INTERNAL 0 @@ -124,7 +123,7 @@ typedef long int long_int; # else typedef long long int long_int; # endif -verify (INT_MAX <= TYPE_MAXIMUM (long_int) / 4 / 366 / 24 / 60 / 60); +static_assert (INT_MAX <= TYPE_MAXIMUM (long_int) / 4 / 366 / 24 / 60 / 60); /* Shift A right by B bits portably, by dividing A by 2**B and truncating towards minus infinity. B should be in the range 0 <= B @@ -157,7 +156,7 @@ static long_int const mktime_max # define EPOCH_YEAR 1970 # define TM_YEAR_BASE 1900 -verify (TM_YEAR_BASE % 100 == 0); +static_assert (TM_YEAR_BASE % 100 == 0); /* Is YEAR + TM_YEAR_BASE a leap year? */ static bool @@ -206,7 +205,7 @@ static long_int ydhms_diff (long_int year1, long_int yday1, int hour1, int min1, int sec1, int year0, int yday0, int hour0, int min0, int sec0) { - verify (-1 / 2 == 0); + static_assert (-1 / 2 == 0); /* Compute intervening leap days correctly even if year is negative. Take care to avoid integer overflow here. */ diff --git a/modules/mktime b/modules/mktime index e5c4c05687..6d924477c5 100644 --- a/modules/mktime +++ b/modules/mktime @@ -10,12 +10,12 @@ Depends-on: time-h c99 multiarch +assert-h[test $REPLACE_MKTIME = 1] intprops[test $REPLACE_MKTIME = 1] libc-config [test $REPLACE_MKTIME = 1] stdbool [test $REPLACE_MKTIME = 1] stdckdint [test $REPLACE_MKTIME = 1] time_r [test $REPLACE_MKTIME = 1] -verify [test $REPLACE_MKTIME = 1] configure.ac: gl_FUNC_MKTIME -- 2.43.0
[PATCH 2/4] mktime: refactor convert_time
This is to better merge with a future version of glibc. This merges a glibc patch by Florian Weimer in: https://sourceware.org/pipermail/libc-alpha/2024-October/160308.html * lib/mktime.c (convert_time): Accept a boolean flag instead of a function pointer. All callers changed. --- ChangeLog | 9 ++ lib/mktime-internal.h | 9 +++--- lib/mktime.c | 71 ++- lib/timegm.c | 2 +- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/ChangeLog b/ChangeLog index 22a40c3d58..08a452130d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2024-10-03 Paul Eggert + + mktime: refactor convert_time + This is to better merge with a future version of glibc. + This merges a glibc patch by Florian Weimer in: + https://sourceware.org/pipermail/libc-alpha/2024-October/160308.html + * lib/mktime.c (convert_time): Accept a boolean flag instead + of a function pointer. All callers changed. + 2024-10-03 Bruno Haible bcp47: Add tests. diff --git a/lib/mktime-internal.h b/lib/mktime-internal.h index 0693aaf140..3e2848c121 100644 --- a/lib/mktime-internal.h +++ b/lib/mktime-internal.h @@ -71,9 +71,8 @@ typedef int mktime_offset_t; #endif /* Subroutine of mktime. Return the time_t representation of TP and - normalize TP, given that a struct tm * maps to a time_t as performed - by FUNC. Record next guess for localtime-gmtime offset in *OFFSET. */ -extern __time64_t __mktime_internal (struct tm *tp, - struct tm *(*func) (__time64_t const *, - struct tm *), + normalize TP, given that a struct tm * maps to a time_t. If + LOCAL, the mapping is performed by localtime_r, otherwise by gmtime_r. + Record next guess for localtime-gmtime offset in *OFFSET. */ +extern __time64_t __mktime_internal (struct tm *tp, bool local, mktime_offset_t *offset) attribute_hidden; diff --git a/lib/mktime.c b/lib/mktime.c index 744a7277bb..92ac9735b0 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -251,29 +251,31 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec, tp->tm_hour, tp->tm_min, tp->tm_sec); } -/* Use CONVERT to convert T to a struct tm value in *TM. T must be in - range for __time64_t. Return TM if successful, NULL (setting errno) on - failure. */ +/* Convert T to a struct tm value in *TM. Use localtime64_r if LOCAL, + otherwise gmtime64_r. T must be in range for __time64_t. Return + TM if successful, NULL (setting errno) on failure. */ static struct tm * -convert_time (struct tm *(*convert) (const __time64_t *, struct tm *), - long_int t, struct tm *tm) +convert_time (bool local, long_int t, struct tm *tm) { __time64_t x = t; - return convert (&x, tm); + if (local) +return __localtime64_r (&x, tm); + else +return __gmtime64_r (&x, tm); } -/* Use CONVERT to convert *T to a broken down time in *TP. - If *T is out of range for conversion, adjust it so that - it is the nearest in-range value and then convert that. - A value is in range if it fits in both __time64_t and long_int. - Return TP on success, NULL (setting errno) on failure. */ +/* Convert *T to a broken down time in *TP (as if by localtime if + LOCAL, otherwise as if by gmtime). If *T is out of range for + conversion, adjust it so that it is the nearest in-range value and + then convert that. A value is in range if it fits in both + __time64_t and long_int. Return TP on success, NULL (setting + errno) on failure. */ static struct tm * -ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *), - long_int *t, struct tm *tp) +ranged_convert (bool local, long_int *t, struct tm *tp) { long_int t1 = (*t < mktime_min ? mktime_min : *t <= mktime_max ? *t : mktime_max); - struct tm *r = convert_time (convert, t1, tp); + struct tm *r = convert_time (local, t1, tp); if (r) { *t = t1; @@ -294,7 +296,7 @@ ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *), long_int mid = long_int_avg (ok, bad); if (mid == ok || mid == bad) break; - if (convert_time (convert, mid, tp)) + if (convert_time (local, mid, tp)) ok = mid, oktm = *tp; else if (errno != EOVERFLOW) return NULL; @@ -310,29 +312,28 @@ ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *), } -/* Convert *TP to a __time64_t value, inverting - the monotonic and mostly-unit-linear conversion function CONVERT. - Use *OFFSET to keep track of a guess at the offset of the result, +/* Convert *TP to a __time64_t value. If LOCAL, the reverse mapping + is performed as if localtime, otherwise as if by gmtime. Use + *OFFSET to keep track of a guess
[PATCH 3/4] mktime: fix "#" indenting
--- lib/mktime.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/mktime.c b/lib/mktime.c index 92ac9735b0..a4974de63f 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -119,11 +119,11 @@ my_tzset (void) __time64_t values that mktime can generate even on platforms where __time64_t is wider than the int components of struct tm. */ -#if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60 +# if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60 typedef long int long_int; -#else +# else typedef long long int long_int; -#endif +# endif verify (INT_MAX <= TYPE_MAXIMUM (long_int) / 4 / 366 / 24 / 60 / 60); /* Shift A right by B bits portably, by dividing A by 2**B and @@ -155,8 +155,8 @@ static long_int const mktime_max = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t) ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t)); -#define EPOCH_YEAR 1970 -#define TM_YEAR_BASE 1900 +# define EPOCH_YEAR 1970 +# define TM_YEAR_BASE 1900 verify (TM_YEAR_BASE % 100 == 0); /* Is YEAR + TM_YEAR_BASE a leap year? */ @@ -172,9 +172,9 @@ leapyear (long_int year) } /* How many days come before each month (0-12). */ -#ifndef _LIBC +# ifndef _LIBC static -#endif +# endif const unsigned short int __mon_yday[2][13] = { /* Normal years. */ -- 2.43.0
[PATCH 1/4] mktime: improve comment wording
* src/mktime.c: Improve comment wording. glibc has changed the comment wording, and this improves on the glibc changes with the result scheduled to be copied back to glibc. --- lib/mktime.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/mktime.c b/lib/mktime.c index c704f41574..744a7277bb 100644 --- a/lib/mktime.c +++ b/lib/mktime.c @@ -536,9 +536,8 @@ __mktime_internal (struct tm *tp, __time64_t __mktime64 (struct tm *tp) { - /* POSIX.1 8.1.1 requires that whenever mktime() is called, the - time zone names contained in the external variable 'tzname' shall - be set as if the tzset() function had been called. */ + /* POSIX.1 requires mktime to set external variables like 'tzname' + as though tzset had been called. */ __tzset (); # if defined _LIBC || NEED_MKTIME_WORKING -- 2.43.0
[PATCH] Mention WG14 N3322 in manual
--- doc/gnulib-readme.texi | 5 + 1 file changed, 5 insertions(+) diff --git a/doc/gnulib-readme.texi b/doc/gnulib-readme.texi index eafb3a0b01..876a495103 100644 --- a/doc/gnulib-readme.texi +++ b/doc/gnulib-readme.texi @@ -547,6 +547,11 @@ hosts. @item Adding zero to a null pointer does not change the pointer. For example, @code{0 + (char *) NULL == (char *) NULL}. +Similarly, subtracting zero does not change a null pointer, +and subtracting two null pointers results in zero. +A future C standard is planned to require this behavior; see +``@url{https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf, +Allow zero length operations on null pointers}'', WG14 N3322 (2024-08-28). @end itemize @end itemize -- 2.43.0
[PATCH 1/2] dirent: define DT_* macros on all platforms
* lib/dirent.in.h (DT_UNKNOWN, DT_FIFO, DT_CHR, DT_DIR, DT_BLK) (DT_REG, DT_LNK, DT_SOCK, DT_WHT): Define these on all platforms, if the system does not already define them. Check that they have distinct values. (_GL_DIRENT_S_ISWHT, _GL_DIRENT_S_IFWHT) [!(IFTODT && DTTOIF)]: New macros. (IFTODT, DTTOIF): Define if not already defined. * modules/dirent (Depends-on): Add assert-h, extensions. --- ChangeLog | 12 + doc/posix-headers/dirent.texi | 23 - lib/dirent.in.h | 89 +++ modules/dirent| 2 + 4 files changed, 114 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3e23303eaa..fc6d4002ef 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2024-10-02 Paul Eggert + + dirent: define DT_* macros on all platforms + * lib/dirent.in.h (DT_UNKNOWN, DT_FIFO, DT_CHR, DT_DIR, DT_BLK) + (DT_REG, DT_LNK, DT_SOCK, DT_WHT): Define these on all platforms, + if the system does not already define them. Check that they + have distinct values. + (_GL_DIRENT_S_ISWHT, _GL_DIRENT_S_IFWHT) [!(IFTODT && DTTOIF)]: + New macros. + (IFTODT, DTTOIF): Define if not already defined. + * modules/dirent (Depends-on): Add assert-h, extensions. + 2024-10-02 Bruno Haible Silence -Wunused-const-variable warnings in Gnulib code. diff --git a/doc/posix-headers/dirent.texi b/doc/posix-headers/dirent.texi index 42dba5f66a..095fc8e9de 100644 --- a/doc/posix-headers/dirent.texi +++ b/doc/posix-headers/dirent.texi @@ -11,6 +11,21 @@ Portability problems fixed by Gnulib: This header file is missing on some platforms: MSVC 14. +@item +Although many systems define directory entry type macros like @code{DT_DIR}, +some do not: +Minix 3.1.8, AIX 7.2, HP-UX 11, Solaris 11.4, mingw. + +@item +The directory entry type macro @code{DT_WHT} is missing on many systems: +All systems where @code{DT_DIR} is missing, plus OpenBSD 7.5. + +@item +The conversion macros @code{DTTOIF} and @code{IFTODT} are missing on +many systems (however, the Gnulib replacement macros +may evaluate their arguments multiple times): +All systems where @code{DT_DIR} is missing, plus OpenBSD 7.5. + @item The type @code{ino_t} is missing on some platforms: glibc 2.23 and others. @@ -21,14 +36,18 @@ Portability problems not fixed by Gnulib: @itemize @item Although many systems define a @code{struct dirent} member named -@code{d_type} and directory entry type macros like @code{DT_DIR} and -@code{DT_LNK}, some do not: +@code{d_type}, some do not: Minix 3.1.8, AIX 7.2, HP-UX 11, Solaris 11.4, mingw. @item On systems with @code{d_type}, not every filesystem supports @code{d_type}, and those lacking support will set it to @code{DT_UNKNOWN}. +@item +The POSIX.1-2024 directory entry type macros @code{DT_MQ}, +@code{DT_SEM}, @code{DT_SHM} and @code{DT_TMO} are missing on many +platforms. + @item Some systems define a @code{struct dirent} member named @code{d_namlen} containing the string length of @code{d_name}, but others do not: diff --git a/lib/dirent.in.h b/lib/dirent.in.h index 7ba8fc64d8..c8c39c3e6b 100644 --- a/lib/dirent.in.h +++ b/lib/dirent.in.h @@ -46,20 +46,89 @@ struct dirent char d_type; char d_name[1]; }; -/* Possible values for 'd_type'. */ -# define DT_UNKNOWN 0 -# define DT_FIFO1 /* FIFO */ -# define DT_CHR 2 /* character device */ -# define DT_DIR 4 /* directory */ -# define DT_BLK 6 /* block device */ -# define DT_REG 8 /* regular file */ -# define DT_LNK10 /* symbolic link */ -# define DT_SOCK 12 /* socket */ -# define DT_WHT14 /* whiteout */ # define GNULIB_defined_struct_dirent 1 # endif #endif +/* 'd_type' macros specified in GNU, i.e., POSIX.1-2024 plus DT_WHT, + but not (yet) DT_MQ, DT_SEM, DT_SHM, DT_TMO. + These macros can be useful even on platforms that do not support + d_type or the corresponding file types. + Default to the Linux values which are also popular elsewhere, + and check that all macros have distinct values. */ +#ifndef DT_UNKNOWN +# define DT_UNKNOWN 0 +#endif +#ifndef DT_FIFO +# define DT_FIFO 1 /* FIFO */ +#endif +#ifndef DT_CHR +# define DT_CHR 2 /* character device */ +#endif +#ifndef DT_DIR +# define DT_DIR 4 /* directory */ +#endif +#ifndef DT_BLK +# define DT_BLK 6 /* block device */ +#endif +#ifndef DT_REG +# define DT_REG 8 /* regular file */ +#endif +#ifndef DT_LNK +# define DT_LNK 10 /* symbolic link */ +#endif +#ifndef DT_SOCK +# define DT_SOCK 12 /* socket */ +#endif +#ifndef DT_WHT +# define DT_WHT 14 /* whiteout */ +#endif +static_assert (DT_UNKNOWN != DT_FIFO && DT_UNKNOWN != DT_CHR + && DT_UNKNOWN != DT_BLK && DT_UNKNOWN != DT_REG + && DT_UNKNOWN != DT_LNK && DT_UNKNOWN !
[PATCH 2/2] file-has-acl: no need for struct stat
Change the API of the new file_has_aclinfo function so that it no longer needs a struct stat *. In some cases this can help GNU ls avoid an unnecessary ‘stat’ call for each file it lists, which can be a significant win. * lib/acl.h (ACL_SYMLINK_NOFOLLOW): Change from 1 to UCHAR_MAX+1 so that it can be ORed with any d_type value. * lib/file-has-acl.c: Include dirent.h, for the DT_* macros. Check that ACL_SYMLINK_NOFOLLOW is outside unsigned char range. (file_has_aclinfo): Change API so that struct stat is no longer needed. Instead, the file type (if known) is passed as part of the flags. All callers changed. Simplify NFSv4 code. * modules/file-has-acl (Depends-on): Add assert-h for static_assert, and dirent for DT_* macros. * tests/test-file-has-acl.c: Include dirent.h. (main): Adjust to file_has_aclinfo API change. Briefly test ACL_SYMLINK_FOLLOW. --- ChangeLog | 18 NEWS | 3 ++ lib/acl.h | 7 +-- lib/file-has-acl.c| 95 --- modules/file-has-acl | 2 + tests/test-file-has-acl.c | 13 +- 6 files changed, 88 insertions(+), 50 deletions(-) diff --git a/ChangeLog b/ChangeLog index fc6d4002ef..d3311a6ea8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2024-10-02 Paul Eggert + file-has-acl: no need for struct stat + Change the API of the new file_has_aclinfo function so that it no + longer needs a struct stat *. In some cases this can help GNU ls + avoid an unnecessary ‘stat’ call for each file it lists, which + can be a significant win. + * lib/acl.h (ACL_SYMLINK_NOFOLLOW): Change from 1 to UCHAR_MAX+1 + so that it can be ORed with any d_type value. + * lib/file-has-acl.c: Include dirent.h, for the DT_* macros. + Check that ACL_SYMLINK_NOFOLLOW is outside unsigned char range. + (file_has_aclinfo): Change API so that struct stat is no longer + needed. Instead, the file type (if known) is passed as part of + the flags. All callers changed. Simplify NFSv4 code. + * modules/file-has-acl (Depends-on): Add assert-h for static_assert, + and dirent for DT_* macros. + * tests/test-file-has-acl.c: Include dirent.h. + (main): Adjust to file_has_aclinfo API change. + Briefly test ACL_SYMLINK_FOLLOW. + dirent: define DT_* macros on all platforms * lib/dirent.in.h (DT_UNKNOWN, DT_FIFO, DT_CHR, DT_DIR, DT_BLK) (DT_REG, DT_LNK, DT_SOCK, DT_WHT): Define these on all platforms, diff --git a/NEWS b/NEWS index c3710825dd..bd120be0b3 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,9 @@ User visible incompatible changes DateModules Changes +2024-10-02 file-has-aclThe file_has_aclinfo function introduced 3 days ago +now has a different signature. + 2024-09-25 string-buffer The function sb_append is renamed to sb_append_c. The function sb_dupfree is renamed to sb_dupfree_c. diff --git a/lib/acl.h b/lib/acl.h index 1a627323ec..29ec2496fa 100644 --- a/lib/acl.h +++ b/lib/acl.h @@ -32,8 +32,9 @@ extern "C" { #endif -/* Follow symlinks when getting an ACL. */ -enum { ACL_SYMLINK_FOLLOW = 1 }; +/* Follow symlinks when getting an ACL. This is greater than any + unsigned char so that it can be ORed with any DT_* value. */ +enum { ACL_SYMLINK_FOLLOW = 1 + (unsigned char) -1 }; /* Information about an ACL. */ struct aclinfo @@ -75,7 +76,7 @@ struct aclinfo bool acl_errno_valid (int) _GL_ATTRIBUTE_CONST; int file_has_acl (char const *, struct stat const *); -int file_has_aclinfo (char const *, struct stat const *, struct aclinfo *, int); +int file_has_aclinfo (char const *restrict, struct aclinfo *restrict, int); #if USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR bool aclinfo_has_xattr (struct aclinfo const *, char const *) diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index f29414de67..9f4213702f 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -27,10 +27,15 @@ #include "acl.h" +#include + #include "acl-internal.h" #include "attribute.h" #include "minmax.h" +/* Check the assumption that UCHAR_MAX < INT_MAX. */ +static_assert (ACL_SYMLINK_FOLLOW & ~ (unsigned char) -1); + static char const UNKNOWN_SECURITY_CONTEXT[] = "?"; #if USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR @@ -321,16 +326,18 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes) 0 if ACLs are not supported, or if NAME has no or only a base ACL, and -1 (setting errno) on error. Note callers can determine if ACLs are not supported as errno is set in that case also. - SB must be set to the stat buffer of NAME, - obtained through stat() or lstat(). - Set *AI to the ACL info if available. - If FLAGS & AT_SYMLINK_FOLLOW, SB was gotten via stat, otherwise
Re: [PATCH] file-has-acl: new function file_has_aclinfo
On 2024-10-01 15:20, Bruno Haible wrote: Why include ? I see nothing that uses it. Oh, one of my working versions used it and I forgot to remove it. Thanks for reporting that. I removed that include.
Re: [PATCH] file-has-acl: new function file_has_aclinfo
On 2024-10-01 14:42, Bruno Haible wrote: ../../gllib/file-has-acl.c:427:21: error: use of undeclared identifier 'acl_extended_file' ? acl_extended_file Thanks, fixed by installing the attached.From 9256d97e074bbae33bbd188e742daccfb72022de Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 1 Oct 2024 19:51:50 -0700 Subject: [PATCH] file-has-acl.c: port back to macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2024-10/msg0.html * lib/file-has-acl.c (file_has_aclinfo): Don’t attempt to link to acl_extended_file if !HAVE_ACL_EXTENDED_FILE. Most of this patch consists of indenting changes; the real change is to use ‘#if’ rather than ‘if’. --- ChangeLog | 10 + lib/file-has-acl.c | 109 + 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7d88adbb21..75f8a2a541 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2024-10-01 Paul Eggert + + file-has-acl.c: port back to macOS + Problem reported by Bruno Haible in: + https://lists.gnu.org/r/bug-gnulib/2024-10/msg0.html + * lib/file-has-acl.c (file_has_aclinfo): Don’t attempt to link to + acl_extended_file if !HAVE_ACL_EXTENDED_FILE. Most of this patch + consists of indenting changes; the real change is to use ‘#if’ + rather than ‘if’. + 2024-10-01 Bruno Haible localename-unsafe: Modernize locale name mapping. diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index 944f0d2200..f29414de67 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -418,76 +418,71 @@ file_has_aclinfo (char const *name, struct stat const *sb, /* Linux, FreeBSD, Mac OS X, IRIX, Tru64, Cygwin >= 2.5 */ int ret; - if (HAVE_ACL_EXTENDED_FILE) /* Linux */ +# if HAVE_ACL_EXTENDED_FILE /* Linux */ +/* On Linux, acl_extended_file is an optimized function: It only + makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for + ACL_TYPE_DEFAULT. */ + ret = ((flags & ACL_SYMLINK_FOLLOW + ? acl_extended_file + : acl_extended_file_nofollow) + (name)); +# elif HAVE_ACL_TYPE_EXTENDED /* Mac OS X */ + /* On Mac OS X, acl_get_file (name, ACL_TYPE_ACCESS) + and acl_get_file (name, ACL_TYPE_DEFAULT) + always return NULL / EINVAL. There is no point in making + these two useless calls. The real ACL is retrieved through + acl_get_file (name, ACL_TYPE_EXTENDED). */ + acl_t acl = acl_get_file (name, ACL_TYPE_EXTENDED); + if (acl) { - /* On Linux, acl_extended_file is an optimized function: It only - makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for - ACL_TYPE_DEFAULT. */ - ret = ((flags & ACL_SYMLINK_FOLLOW - ? acl_extended_file - : acl_extended_file_nofollow) - (name)); + ret = acl_extended_nontrivial (acl); + acl_free (acl); } - else /* FreeBSD, Mac OS X, IRIX, Tru64, Cygwin >= 2.5 */ -{ -# if HAVE_ACL_TYPE_EXTENDED /* Mac OS X */ - /* On Mac OS X, acl_get_file (name, ACL_TYPE_ACCESS) - and acl_get_file (name, ACL_TYPE_DEFAULT) - always return NULL / EINVAL. There is no point in making - these two useless calls. The real ACL is retrieved through - acl_get_file (name, ACL_TYPE_EXTENDED). */ - acl_t acl = acl_get_file (name, ACL_TYPE_EXTENDED); - if (acl) -{ - ret = acl_extended_nontrivial (acl); - acl_free (acl); -} - else -ret = -1; + else +ret = -1; # else /* FreeBSD, IRIX, Tru64, Cygwin >= 2.5 */ - acl_t acl = acl_get_file (name, ACL_TYPE_ACCESS); - if (acl) -{ - int saved_errno; + acl_t acl = acl_get_file (name, ACL_TYPE_ACCESS); + if (acl) +{ + int saved_errno; - ret = acl_access_nontrivial (acl); - saved_errno = errno; - acl_free (acl); - errno = saved_errno; + ret = acl_access_nontrivial (acl); + saved_errno = errno; + acl_free (acl); + errno = saved_errno; # if HAVE_ACL_FREE_TEXT /* Tru64 */ - /* On OSF/1, acl_get_file (name, ACL_TYPE_DEFAULT) always - returns NULL with errno not set. There is no point in - making this call. */ + /* On OSF/1, acl_get_file (name, ACL_TYPE_DEFAULT) always + returns NULL with errno not set. There is no point in + making this call. */ # else /* FreeBSD, IRIX, Cygwin >= 2.5 */ - /* On Linux, FreeBSD, IRIX, acl_get_file (name, ACL_TYPE_ACCESS) -
Re: [PATCH] file-has-acl: new function file_has_aclinfo
On 2024-09-30 06:30, Bruno Haible wrote: Did you mean 152 ? Or maybe 512 ? I used "152" because that is what file-has-acl previously used. I installed the first patch to explain this better. I installed the 2nd patch as an optimization discovered after more testing with coreutils. Further improvements are in the pipeline to fix some coreutils bugs I introduced with all this. These will involve using dirent stuff. Speaking of dirent, I see that POSIX.1-2024 has a new function posix_getdents that a quick web search suggests is only in Cygwin and musl now. It would simplify parts of Gnulib, I think. But it can wait.From 0c55c3d7f74d42d273e0fffc8b774344eed65458 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 30 Sep 2024 09:24:09 -0700 Subject: [PATCH 1/2] file-has-acl: improve acl.h comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/acl.h: Improve comment about ‘152’. --- lib/acl.h | 5 - lib/file-has-acl.c | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/acl.h b/lib/acl.h index 07ab04250d..1a627323ec 100644 --- a/lib/acl.h +++ b/lib/acl.h @@ -65,7 +65,10 @@ struct aclinfo int err; /* A small array of char, big enough for most listxattr results. - The size is somewhat arbitrary. For internal use only. */ + The size is somewhat arbitrary; it equals the max length of a + trivial NFSv4 ACL (a size used by file-has-acl.c in 2023-2024 + but no longer relevant now), and a different value might be + better once experience is gained. For internal use only. */ char __gl_acl_ch[152]; } u; }; diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index 89faba565f..6924a616bd 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -349,8 +349,7 @@ file_has_aclinfo (char const *name, struct stat const *sb, || (S_ISDIR (sb->st_mode) && aclinfo_has_xattr (ai, XATTR_NAME_POSIX_ACL_DEFAULT; - /* If there is an NFSv4 ACL, follow up with a getxattr syscall - to see whether the NFSv4 ACL is nontrivial. */ + /* If there is an NFSv4 ACL, check whether it is nontrivial. */ if (nfsv4_acl) { /* A buffer large enough to hold any trivial NFSv4 ACL. -- 2.43.0 From 276260e7ec35181b0ea9fddf5bab397cf1061eca Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 30 Sep 2024 10:46:57 -0700 Subject: [PATCH 2/2] file-has-acl: improve performance on Linux symlinks * lib/file-has-acl.c (USE_LINUX_XATTR): Now can be used outside #if. (file_has_aclinfo): As an optimization on Linux, do not attempt to get extended attributes on symlinks, as this will always fail. Also, use lgetxattr (not getxattr) and acl_extended_file_nofollow (not acl_extended_file) when not following symlinks, to avoid some races. --- ChangeLog | 10 lib/file-has-acl.c | 143 ++--- 2 files changed, 92 insertions(+), 61 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0679ed28ef..f971059a0a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2024-09-30 Paul Eggert + + file-has-acl: improve performance on Linux symlinks + * lib/file-has-acl.c (USE_LINUX_XATTR): Now can be used outside #if. + (file_has_aclinfo): As an optimization on Linux, do not attempt to + get extended attributes on symlinks, as this will always fail. + Also, use lgetxattr (not getxattr) and acl_extended_file_nofollow + (not acl_extended_file) when not following symlinks, to avoid some + races. + 2024-09-30 Bruno Haible selinux-h: Fix two syntax errors. diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index 6924a616bd..944f0d2200 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -33,7 +33,11 @@ static char const UNKNOWN_SECURITY_CONTEXT[] = "?"; -#define USE_LINUX_XATTR (USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR) +#if USE_ACL && HAVE_LINUX_XATTR_H && HAVE_LISTXATTR +# define USE_LINUX_XATTR true +#else +# define USE_LINUX_XATTR false +#endif #if USE_LINUX_XATTR # if USE_SELINUX_SELINUX_H @@ -320,80 +324,95 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes) SB must be set to the stat buffer of NAME, obtained through stat() or lstat(). Set *AI to the ACL info if available. - If FLAGS & AT_SYMLINK_FOLLOW, SB was gotten via stat and follow symlinks; - otherwise, SB was gotten vi lstat and do not follow them. */ + If FLAGS & AT_SYMLINK_FOLLOW, SB was gotten via stat, otherwise lstat. + Also, if FLAGS & AT_SYMLINK_FOLLOW, follow symlinks when retrieving + ACL info, otherwise do not follow them if possible. */ int file_has_aclinfo (char const *name, struct stat const *sb, struct aclinfo *ai, int flags) { -#if USE_LINUX_XATTR - int initial_errno = errno; - get_aclinfo (name, ai, flags); + /* Symbolic links lack extended attr
[PATCH] file-has-acl: new function file_has_aclinfo
This is for GNU coreutils, whose ‘ls’ can be made faster with the new function because it means (for example) that ‘ls’ needn’t call both listxattr and lgetxattr in the common case where the file has no extended attributes. * lib/acl.h (ACL_SYMLINK_FOLLOW): New constant. (struct aclinfo): New type. * lib/file-has-acl.c (UNKNOWN_SECURITY_CONTEXT): New constant. (USE_LINUX_XATTR): New macro. When set: Include stdint.h (for uint32_t). If also USE_SELINUX_SELINUX_H, include selinux/selinux.h (for getfilecon). If HAVE_SMACK, include , otherwise provide substitutes for smack_smackfs_path and smack_new_label_from_path. (is_smack_enabled): New function. (aclinfo_has_xattr): Rename from have_xattr and make it extern. Use struct aclinfo const * arg instead of char const * and ssize_t. Work even if ai->size < 0. Supply a no-op macro on platforms lacking xattr. (get_aclinfo): New static function, much of it taken from coreutils/src/ls.c. (file_has_aclinfo): New function, which generalizes file_has_acl to provide more info. Its body is taken from the old file_has_acl with new stuff from ls.c. (file_has_acl): Use it. (aclinfo_scontext_free, aclinfo_free): Nwew functions, if not already no-op macros. * m4/acl.m4 (gl_FUNC_ACL_ARG): Add --without-libsmack option. (gl_FILE_HAS_ACL): Check for libsmack, selinux. * m4/selinux-selinux-h.m4 (gl_CHECK_HEADER_SELINUX_SELINUX_H): New macro, for use by file-has-acl. Rename HAVE_SELINUX_SELINUX_H to USE_SELINUX_SELINUX_H to avoid confusion as to whether we have ; all uses changed. (gl_HEADERS_SELINUX_SELINUX_H): Use the new macro. * modules/file-has-acl (Files): Add m4/selinux-selinux-h.m4. (Depends-on): Add errno, ssize_t. * tests/test-file-has-acl.c (main): Add a little test for file_has_aclinfo. --- ChangeLog | 40 + lib/acl.h | 53 +++ lib/file-has-acl.c| 310 -- lib/se-selinux.in.h | 2 +- m4/acl.m4 | 39 - m4/selinux-selinux-h.m4 | 34 +++-- modules/file-has-acl | 3 + modules/selinux-h | 2 +- tests/test-file-has-acl.c | 11 ++ 9 files changed, 398 insertions(+), 96 deletions(-) diff --git a/ChangeLog b/ChangeLog index 071f0f0fc7..3368abacb4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,43 @@ +2024-09-29 Paul Eggert + + file-has-acl: new function file_has_aclinfo + This is for GNU coreutils, whose ‘ls’ can be made faster with the + new function because it means (for example) that ‘ls’ needn’t call + both listxattr and lgetxattr in the common case where the file has + no extended attributes. + * lib/acl.h (ACL_SYMLINK_FOLLOW): New constant. + (struct aclinfo): New type. + * lib/file-has-acl.c (UNKNOWN_SECURITY_CONTEXT): New constant. + (USE_LINUX_XATTR): New macro. When set: + Include stdint.h (for uint32_t). If also USE_SELINUX_SELINUX_H, + include selinux/selinux.h (for getfilecon). + If HAVE_SMACK, include , otherwise provide substitutes + for smack_smackfs_path and smack_new_label_from_path. + (is_smack_enabled): New function. + (aclinfo_has_xattr): Rename from have_xattr and make it extern. + Use struct aclinfo const * arg instead of char const * and ssize_t. + Work even if ai->size < 0. Supply a no-op macro on platforms + lacking xattr. + (get_aclinfo): New static function, much of it taken from + coreutils/src/ls.c. + (file_has_aclinfo): New function, which generalizes file_has_acl + to provide more info. Its body is taken from the old file_has_acl + with new stuff from ls.c. + (file_has_acl): Use it. + (aclinfo_scontext_free, aclinfo_free): Nwew functions, if not + already no-op macros. + * m4/acl.m4 (gl_FUNC_ACL_ARG): Add --without-libsmack option. + (gl_FILE_HAS_ACL): Check for libsmack, selinux. + * m4/selinux-selinux-h.m4 (gl_CHECK_HEADER_SELINUX_SELINUX_H): + New macro, for use by file-has-acl. Rename HAVE_SELINUX_SELINUX_H + to USE_SELINUX_SELINUX_H to avoid confusion as to whether we have + ; all uses changed. + (gl_HEADERS_SELINUX_SELINUX_H): Use the new macro. + * modules/file-has-acl (Files): Add m4/selinux-selinux-h.m4. + (Depends-on): Add errno, ssize_t. + * tests/test-file-has-acl.c (main): Add a little test for + file_has_aclinfo. + 2024-09-26 Bruno Haible string-buffer tests: Avoid test failure on native Windows. diff --git a/lib/acl.h b/lib/acl.h index 475231c2db..07ab04250d 100644 --- a/lib/acl.h +++ b/lib/acl.h @@ -32,9 +32,62 @@ extern "C" { #endif +/* Follow symlinks when getting an ACL. */ +enum { ACL_SYMLINK_FOLLOW = 1 }; + +/* Information about an ACL. */ +struct aclinfo +{ + /* If 'size' is nonnegative, a buffer holding the concatenation + of extended attribute names, each terminated by NUL + (either u.__gl_acl_ch,
[PATCH] sigsegv-tests: port to GCC 14
GCC 14 on x86-64 with -O2 apparently outsmarts our test for null pointer dereference, and this is something the C standard allows. Fix the test by putting ‘volatile’ at the right place. * tests/test-sigsegv-catch-stackoverflow2.c (null_pointer): Make it a volatile pointer, not a pointer to volatile. Also, rename from null_pointer_to_volatile_int; use changed. --- ChangeLog | 10 ++ tests/test-sigsegv-catch-stackoverflow2.c | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index e6dffe82e6..65467fbfe5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2024-09-20 Paul Eggert + + sigsegv-tests: port to GCC 14 + GCC 14 on x86-64 with -O2 apparently outsmarts our test for null + pointer dereference, and this is something the C standard allows. + Fix the test by putting ‘volatile’ at the right place. + * tests/test-sigsegv-catch-stackoverflow2.c (null_pointer): + Make it a volatile pointer, not a pointer to volatile. + Also, rename from null_pointer_to_volatile_int; use changed. + 2024-09-20 Bruno Haible jit/cache tests: Fix crash with clang's UBSAN + ASAN. diff --git a/tests/test-sigsegv-catch-stackoverflow2.c b/tests/test-sigsegv-catch-stackoverflow2.c index 60f7eb0cfb..133b616af2 100644 --- a/tests/test-sigsegv-catch-stackoverflow2.c +++ b/tests/test-sigsegv-catch-stackoverflow2.c @@ -59,7 +59,7 @@ static sigset_t mainsigset; static volatile int pass = 0; static uintptr_t page; -static volatile int *null_pointer_to_volatile_int /* = NULL */; +static int *volatile null_pointer /* = NULL */; static void stackoverflow_handler_continuation (void *arg1, void *arg2, void *arg3) @@ -193,7 +193,7 @@ main () *(volatile int *) (page + 0x678) = 42; break; case 3: - *null_pointer_to_volatile_int = 42; + *null_pointer = 42; break; case 4: break; -- 2.46.0
Re: Merge fnmatch and fnmatch-gnu?
On 2024-09-19 20:14, Collin Funk wrote: Or should fnmatch be changed to support FNM_CASEFOLD but not FNM_EXTMATCH, which is still a GNU extension? Can you remind me why we have the two modules? If that reason still applies, we should follow the suggestion above; otherwise I suppose we should have just one module.
[PATCH] utimens: port to NetBSD-10.99.12/amd64
On this platform, declares utimens and lutimens and the C library defines them, so we needn’t (and shouldn’t). Problem reported privately by Thomas Klausner. * lib/utimens.c (utimens) [HAVE_UTIMENS]: Don’t define. (lutimens) [HAVE_LUTIMENS]: Don’t define. * lib/utimens.h [HAVE_UTIMENS || HAVE_LUTIMENS]: Include , for NetBSD’s declaration of utimens and lutimens. (utimens) [HAVE_UTIMENS]: Don’t declare. (lutimens) [HAVE_LUTIMENS]: Don’t declare. * m4/utimens.m4 (gl_UTIMENS): Check for utimens, lutimens. --- ChangeLog | 14 ++ lib/utimens.c | 4 lib/utimens.h | 8 m4/utimens.m4 | 3 ++- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 9ba5d0a184..1270ac58c8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2024-09-16 Paul Eggert + + utimens: port to NetBSD-10.99.12/amd64 + On this platform, declares utimens and lutimens and the + C library defines them, so we needn’t (and shouldn’t). + Problem reported privately by Thomas Klausner. + * lib/utimens.c (utimens) [HAVE_UTIMENS]: Don’t define. + (lutimens) [HAVE_LUTIMENS]: Don’t define. + * lib/utimens.h [HAVE_UTIMENS || HAVE_LUTIMENS]: + Include , for NetBSD’s declaration of utimens and lutimens. + (utimens) [HAVE_UTIMENS]: Don’t declare. + (lutimens) [HAVE_LUTIMENS]: Don’t declare. + * m4/utimens.m4 (gl_UTIMENS): Check for utimens, lutimens. + 2024-09-16 Bruno Haible stdlib: Fix compilation error with Sun C++. diff --git a/lib/utimens.c b/lib/utimens.c index 6b9f62a53c..cd86a44ea7 100644 --- a/lib/utimens.c +++ b/lib/utimens.c @@ -516,6 +516,7 @@ fdutimens (int fd, char const *file, struct timespec const timespec[2]) } } +#if !HAVE_UTIMENS /* Set the access and modification timestamps of FILE to be TIMESPEC[0] and TIMESPEC[1], respectively. */ int @@ -523,7 +524,9 @@ utimens (char const *file, struct timespec const timespec[2]) { return fdutimens (-1, file, timespec); } +#endif +#if !HAVE_LUTIMENS /* Set the access and modification timestamps of FILE to be TIMESPEC[0] and TIMESPEC[1], respectively, without dereferencing symlinks. Fail with ENOSYS if the platform does not support @@ -646,3 +649,4 @@ lutimens (char const *file, struct timespec const timespec[2]) errno = ENOSYS; return -1; } +#endif diff --git a/lib/utimens.h b/lib/utimens.h index b20d4f4f7e..e85477b849 100644 --- a/lib/utimens.h +++ b/lib/utimens.h @@ -24,13 +24,21 @@ #include +#if HAVE_UTIMENS || HAVE_LUTIMENS +# include +#endif + #ifdef __cplusplus extern "C" { #endif int fdutimens (int, char const *, struct timespec const [2]); +#if !HAVE_UTIMENS int utimens (char const *, struct timespec const [2]); +#endif +#if !HAVE_LUTIMENS int lutimens (char const *, struct timespec const [2]); +#endif #ifdef __cplusplus } diff --git a/m4/utimens.m4 b/m4/utimens.m4 index 9996e3ef33..b8200deaa2 100644 --- a/m4/utimens.m4 +++ b/m4/utimens.m4 @@ -1,5 +1,5 @@ # utimens.m4 -# serial 16 +# serial 17 dnl Copyright (C) 2003-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -16,6 +16,7 @@ AC_DEFUN([gl_UTIMENS], gl_CHECK_FUNCS_ANDROID([lutimes], [[#include ]]) gl_CHECK_FUNCS_ANDROID([futimens], [[#include ]]) gl_CHECK_FUNCS_ANDROID([utimensat], [[#include ]]) + AC_CHECK_FUNCS_ONCE([utimens lutimens]) if test $ac_cv_func_futimens = no && test $ac_cv_func_futimesat = yes; then dnl FreeBSD 8.0-rc2 mishandles futimesat(fd,NULL,time). It is not -- 2.43.0
[PATCH] fflush: NetBSD, OpenBSD can’t fflush input
* m4/fflush.m4 (gl_FUNC_FFLUSH_STDIN): Guess no on NetBSD and OpenBSD; they document fflush to fail unless the stream is open for writing. --- ChangeLog | 7 +++ doc/posix-functions/fflush.texi | 3 +++ m4/fflush.m4| 11 ++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index b2f3e3ffb6..d65e8cb88d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2024-09-16 Paul Eggert + + fflush: NetBSD, OpenBSD can’t fflush input + * m4/fflush.m4 (gl_FUNC_FFLUSH_STDIN): Guess no on NetBSD and + OpenBSD; they document fflush to fail unless the stream is open + for writing. + 2024-09-16 Bruno Haible unictype/category-of: Fix integer overflow in generated table. diff --git a/doc/posix-functions/fflush.texi b/doc/posix-functions/fflush.texi index 0f6b700172..39f8ae9739 100644 --- a/doc/posix-functions/fflush.texi +++ b/doc/posix-functions/fflush.texi @@ -20,6 +20,9 @@ end of the previous buffer, on some platforms: mingw, MSVC 14. @code{fflush} on an input stream right after @code{ungetc} does not discard the @code{ungetc} buffer, on some platforms: macOS 14, FreeBSD 6.0, NetBSD 10.0, OpenBSD 7.5, Cygwin 1.5.25-10. +@item +@code{fflush} fails with @code{EBADF} if the stream is not open for writing: +NetBSD 10.0, OpenBSD 7.5. @end itemize Portability problems not fixed by Gnulib: diff --git a/m4/fflush.m4 b/m4/fflush.m4 index 43fc3bf346..1b30fb91d4 100644 --- a/m4/fflush.m4 +++ b/m4/fflush.m4 @@ -1,5 +1,5 @@ # fflush.m4 -# serial 19 +# serial 20 dnl Copyright (C) 2007-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -79,8 +79,9 @@ AC_DEFUN([gl_FUNC_FFLUSH_STDIN], [gl_cv_func_fflush_stdin=yes], [gl_cv_func_fflush_stdin=no], [case "$host_os" in - # Guess no on native Windows. - mingw* | windows*) gl_cv_func_fflush_stdin="guessing no" ;; + # Guess no on NetBSD, OpenBSD, native Windows. + netbsd* | openbsd* | mingw* | windows*) + gl_cv_func_fflush_stdin="guessing no" ;; *) gl_cv_func_fflush_stdin=cross ;; esac ]) @@ -92,8 +93,8 @@ AC_DEFUN([gl_FUNC_FFLUSH_STDIN], *)gl_func_fflush_stdin='(-1)' ;; esac AC_DEFINE_UNQUOTED([FUNC_FFLUSH_STDIN], [$gl_func_fflush_stdin], -[Define to 1 if fflush is known to work on stdin as per POSIX.1-2008, - 0 if fflush is known to not work, -1 if unknown.]) +[Define to 1 if fflush is known to work on stdin as per POSIX.1-2008 + or later, 0 if fflush is known to not work, -1 if unknown.]) ]) # Prerequisites of lib/fflush.c. -- 2.43.0
Re: [PATCH] parse-datetime no longer depends on nstrftime
Oops, I messed up and committed the wrong file. I fixed that by installing the attached fixup. Sorry about that. From e3a81ab6e1201eb40a9f861c75162b41af148c10 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 16 Sep 2024 11:19:17 -0700 Subject: [PATCH] Fix typo in previous patch --- lib/parse-datetime.y | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/parse-datetime.y b/lib/parse-datetime.y index b036271d01..c27ab60a84 100644 --- a/lib/parse-datetime.y +++ b/lib/parse-datetime.y @@ -38,7 +38,6 @@ #include "idx.h" #include "intprops.h" #include "timespec.h" -#include "strftime.h" /* There's no need to extend the stack, so there's no need to involve alloca. */ -- 2.43.0
[PATCH] parse-datetime no longer depends on nstrftime
I discovered this unnecessary dependency when debugging GNU Patch. * lib/parse-datetime.y: (populate_local_time_zone_table) [!HAVE_STRUCT_TM_TM_ZONE]: (debug_strfdatetime): Use strftime not nstrftime, as we don’t need nstrftime’s extensions or bug fixes. * modules/parse-datetime (Depends-on): Remove nstrftime. Also remove setenv, unsetenv, timegm, as this module no longer depends on them directly. --- ChangeLog | 11 +++ lib/parse-datetime.y | 13 + modules/parse-datetime | 4 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4f3555e6d..a619271ded 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2024-09-16 Paul Eggert + parse-datetime no longer depends on nstrftime + I discovered this unnecessary dependency when debugging + GNU Patch. + * lib/parse-datetime.y: + (populate_local_time_zone_table) [!HAVE_STRUCT_TM_TM_ZONE]: + (debug_strfdatetime): Use strftime not nstrftime, + as we don’t need nstrftime’s extensions or bug fixes. + * modules/parse-datetime (Depends-on): Remove nstrftime. + Also remove setenv, unsetenv, timegm, as this module + no longer depends on them directly. + Don’t port July [[...]] changes to C89 Yesterday’s changes to port to C17 and earlier were intrusive, since they twice replaced one macro with two. Revert the macro diff --git a/lib/parse-datetime.y b/lib/parse-datetime.y index 665401d422..b036271d01 100644 --- a/lib/parse-datetime.y +++ b/lib/parse-datetime.y @@ -1591,7 +1591,7 @@ populate_local_time_zone_table (parser_control *pc, struct tm const *tm) zone = tm->tm_zone; #else char *tz_abbr = pc->tz_abbr[first_entry_exists]; - if (nstrftime (tz_abbr, TIME_ZONE_BUFSIZE, "%Z", tm, 0, 0)) + if (strftime (tz_abbr, TIME_ZONE_BUFSIZE, "%Z", tm)) zone = tz_abbr; #endif e->name = zone; @@ -1613,21 +1613,18 @@ debug_strfdatetime (struct tm const *tm, parser_control const *pc, issues with the parsing - better to avoid formats that could be mis-interpreted (e.g., just -MM-DD). - 2. Can strftime be used instead? -depends if it is portable and can print invalid dates on all systems. + 2. Print timezone information ? - 3. Print timezone information ? + 3. Print DST information ? - 4. Print DST information ? - - 5. Print nanosecond information ? + 4. Print nanosecond information ? NOTE: Printed date/time values might not be valid, e.g., '2016-02-31' or '2016-19-2016' . These are the values as parsed from the user string, before validation. */ - int m = nstrftime (buf, n, "(Y-M-D) %Y-%m-%d %H:%M:%S", tm, 0, 0); + int m = strftime (buf, n, "(Y-M-D) %Y-%m-%d %H:%M:%S", tm); /* If parser_control information was provided (for timezone), and there's enough space in the buffer, add timezone info. */ diff --git a/modules/parse-datetime b/modules/parse-datetime index e8ac3e725d..fc26391aa9 100644 --- a/modules/parse-datetime +++ b/modules/parse-datetime @@ -20,14 +20,10 @@ idx intprops inttypes mktime -nstrftime -setenv stdckdint -unsetenv time-h time_r time_rz -timegm configure.ac: gl_PARSE_DATETIME -- 2.43.0
Re: [PATCH] Port July changes for [[...]] to C17
On 2024-09-16 00:16, Bruno Haible wrote: How about, instead of defining two macros _GL_FUNCDECL_SYS (func, rettype, parameters); _GL_FUNCATTR_SYS (func, rettype, parameters, attributes); we would have one macro _GL_FUNCDECL_SYS (func, rettype, parameters, attributes); and invoke it with 4 arguments always, Thanks, that sounds nicer. I did that. My earlier patch had been assuming we couldn't use that C99 feature; I also documented that we're assuming C99 empty macro args by installing the attached additional patch. The current code doesn't check for portability to strict C17 and earlier, when compiling with recent GCC. Although it could be complicated to do so I hope the complexity isn't necessary, as we can check by compiling with stricter C17- compilers.From 3cbc5ea46fee3a9617bd9478dc8f75bc6cb135ca Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 16 Sep 2024 09:54:08 -0700 Subject: [PATCH] Document use of empty macro args. --- ChangeLog | 2 ++ doc/gnulib-readme.texi | 3 +++ 2 files changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 8bf9e172c5..d4f3555e6d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,6 +8,8 @@ one is empty. Although this change requires C99 or later, that’s safe nowadays. Suggested by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2024-09/msg00079.html + * doc/gnulib-readme.texi (C99 features assumed): + Document that we assume this C99 feature. * lib/c++defs.h (_GL_FUNCDECL_RPL, _GL_FUNCDECL_SYS): Revert yesterday’s changes. All callers changed. Add comments explaining the required comma before missing attrs. diff --git a/doc/gnulib-readme.texi b/doc/gnulib-readme.texi index 25d7b54e2c..eafb3a0b01 100644 --- a/doc/gnulib-readme.texi +++ b/doc/gnulib-readme.texi @@ -376,6 +376,9 @@ it directly, preferring plain @code{bool} via the @item Compound literals and designated initializers. +@item +Empty arguments in macro calls. + @item Variadic macros.@* @findex __VA_ARGS__ -- 2.43.0
Re: [PATCH] Add a missing pure attribute
On 2024-09-15 02:25, Bruno Haible wrote: it can happen that you add the attribute in order to placate one GCC version and then another GCC version complains that you should not have added it. Yes, and if memory serves older GCC versions were pretty bad about this, and that caused me (at least) to add attributes that were in hindsight unnecessary. GCC has gotten better about not diagnosing "missing" attributes for static functions, so these old additions can be removed whenever anyone gets around to it. I typically worry about pacifying only current GCC, as it's more trouble than it's typically worth to work around old GCC bugs merely to suppress false alarms. With older GCC, I suggest configuring with --disable-gcc-warnings or equivalent.
Re: lstatat module?
On 2024-09-13 18:08, Bruno Haible wrote: Why not use fstatat() directly? Yes, lstatat is merely a deprecated convenience function.
Re: malloca.c non-top variable declaration
On 2024-09-11 01:58, Bruno Haible wrote: 1) Gnulib makes use of C99 __VA_ARGS__ in a couple of places. I vaguely recall using other C99 features that can't easily be simulated or finessed by Gnulib. These include designated initializers, compound literals, and the subset of VLAs that are still required in C11-and-later. I'm mentioning these issues because I remember them offhand, and I want to warn Simon that they might be lurking. We still don't use // comments though. That might be the final frontier. :-)
Re: striconv, striconveh, unicodeio: Drop workaround for glibc 2.1
On 2024-09-09 13:38, Jeffrey Walton wrote: Apple is notorious for deprecating support quickly in an effort to drive hardware sales. Sure, but people who run macOS releases before macOS 12 (the oldest Apple still supports) are kinda on their own no matter what we do. Although Gnulib still contains some code for pre-12 macOS, that code is rarely tested and I wouldn't trust it, just as I wouldn't trust Gnulib code written for systems so old that the code is no longer tested. The market determines what support is needed The GNU project relies mostly on volunteer effort and is not a market in the usual sense. We try to direct volunteers to areas where their work will be most helpful to the project. Supporting ancient macOS or MS-Windows versions is generally low the priority list. There are some exceptions. In a few cases (e.g., Emacs) volunteers take the extra effort to test and support old MS-DOS versions, or VMS, or whatever. As long as that doesn't significantly hamper the main project goals, that's a good thing. some products have technical reasons to remain fixed on an older version, like some medical equipment Ancient medical equipment should not, not, *not* install new versions of Emacs!
Re: striconv, striconveh, unicodeio: Drop workaround for glibc 2.1
On 2024-09-09 07:06, Bruno Haible wrote: And more of the same kind. Solaris 9's successor, Solaris 10, was released in 2005. Solaris 9 workarounds therefore can also be dropped. Yes, sounds good. The rule I suggest is that if a distro's supplier no longer supports it, we needn't support it either. Solaris 10 is still supported by Oracle but Solaris 9 is not, so we can drop Solaris 9-and-earlier support.
Re: license of std-gnu11, std-gnu23 modules
On 2024-08-30 04:13, Bruno Haible wrote: Do you agree with that? Yes, that sounds right. I implemented that suggestion by installing the attached.From e87d09bee37eeb742b8a34c9054cd2ebde22b835 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 5 Sep 2024 18:31:29 -0700 Subject: [PATCH] Fix COPYING.EXCEPTION license notices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00227.html * m4/std-gnu11.m4: Fix license notice to use COPYING.EXCEPTION correctly. * m4/std-gnu23.m4: Likewise. Also document the commit ID, since Autoconf was updated likewise and we’re copying from there. --- ChangeLog | 10 ++ m4/std-gnu11.m4 | 19 ++- m4/std-gnu23.m4 | 22 +++--- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 366aa8464c..f87bc0f741 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2024-09-05 Paul Eggert + + Fix COPYING.EXCEPTION license notices + Problem reported by Bruno Haible in: + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00227.html + * m4/std-gnu11.m4: Fix license notice to use COPYING.EXCEPTION + correctly. + * m4/std-gnu23.m4: Likewise. Also document the commit ID, + since Autoconf was updated likewise and we’re copying from there. + 2024-09-04 Bruno Haible Resolve conflicts for functions introduced in Android API level 35. diff --git a/m4/std-gnu11.m4 b/m4/std-gnu11.m4 index e8d5de7a1e..d61d694297 100644 --- a/m4/std-gnu11.m4 +++ b/m4/std-gnu11.m4 @@ -1,5 +1,5 @@ # std-gnu11.m4 -# serial 2 +# serial 3 # Prefer GNU C11 and C++11 to earlier versions. -*- coding: utf-8 -*- @@ -9,6 +9,7 @@ m4_ifndef([_AC_C_C23_OPTIONS], [ # This implementation is taken from GNU Autoconf lib/autoconf/c.m4 # commit 017d5ddd82854911f0119691d91ea8a1438824d6 # dated Sun Apr 3 13:57:17 2016 -0700 +# with minor changes to commentary. # This implementation will be obsolete once we can assume Autoconf 2.70 # or later is installed everywhere a Gnulib program might be developed. @@ -17,9 +18,10 @@ m4_version_prereq([2.70], [], [ # Copyright (C) 2001-2024 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 +# This file is part of Autoconf. 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, @@ -27,8 +29,15 @@ m4_version_prereq([2.70], [], [ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. # +# Under Section 7 of GPL version 3, you are granted additional +# permissions described in the Autoconf Configure Script Exception, +# version 3.0, as published by the Free Software Foundation. +# # You should have received a copy of the GNU General Public License -# along with this program. If not, see <https://www.gnu.org/licenses/>. +# and a copy of the Autoconf Configure Script Exception along with +# this program; see the files COPYINGv3 and COPYING.EXCEPTION +# respectively. If not, see <https://www.gnu.org/licenses/> and +# <https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=blob_plain;f=COPYING.EXCEPTION>. # Written by David MacKenzie, with help from # Akim Demaille, Paul Eggert, diff --git a/m4/std-gnu23.m4 b/m4/std-gnu23.m4 index db65c2bc36..cebf62d283 100644 --- a/m4/std-gnu23.m4 +++ b/m4/std-gnu23.m4 @@ -1,11 +1,11 @@ # std-gnu23.m4 -# serial 1 +# serial 2 # Prefer GNU C23 to earlier versions. # This implementation is taken from GNU Autoconf lib/autoconf/c.m4 -# commit 653956f44683e1daed9827de429590bdd2e317e7 -# dated Tue Apr 30 10:26:50 2024 -0700 +# commit 93b3d33c1d09b05601b240bf17b83f50ac4a148b +# dated Thu Sep 5 18:19:36 2024 -0700 # This implementation will be obsolete once we can assume Autoconf 2.73 # or later is installed everywhere a Gnulib program might be developed. @@ -14,9 +14,10 @@ m4_version_prereq([2.73], [], [ # Copyright (C) 2001-2024 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 +# This file is part of Autoconf. 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
Re: savewd: please port to native Windows
On 2024-08-27 14:31, Bruno Haible wrote: Maybe instead define it as a macro? #define fork() (assure (false), -1) Sure, I installed it that way.
Re: savewd: please port to native Windows
On 2024-08-27 13:34, Bruno Haible wrote: gllib\savewd.c(73,27): warning: implicit declaration of function 'fork' is invalid in C99 [-Wimplicit-function-declaration] Perhaps the attached patch?From 4e624ff79120231a78b85a2302d037e205c69dd3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 27 Aug 2024 14:23:59 -0700 Subject: [PATCH] savewd: port to native MS-Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/savewd.c (fork) [_WIN32 && !__CYGWIN]: New static function. (savewd_save): Don’t fork on MS-Windows, or if O_SEARCH != O_RDONLY. --- ChangeLog| 6 ++ lib/savewd.c | 13 - 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 767f51f9d3..d80aa2107d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2024-08-27 Paul Eggert + + savewd: port to native MS-Windows + * lib/savewd.c (fork) [_WIN32 && !__CYGWIN]: New static function. + (savewd_save): Don’t fork on MS-Windows, or if O_SEARCH != O_RDONLY. + 2024-08-27 Bruno Haible write-any-file: Don't reference an undefined function on native Windows. diff --git a/lib/savewd.c b/lib/savewd.c index 57692c0b3d..7c5ba17c42 100644 --- a/lib/savewd.c +++ b/lib/savewd.c @@ -36,6 +36,10 @@ #include "fcntl-safer.h" #include "filename.h" +#if defined _WIN32 && !defined __CYGWIN__ +static pid_t fork (void) { assure (false); } +#endif + /* Save the working directory into *WD, if it hasn't been saved already. Return true if a child has been forked to do the real work. */ @@ -54,7 +58,14 @@ savewd_save (struct savewd *wd) wd->val.fd = fd; break; } -if (errno != EACCES && errno != ESTALE) +# if O_SEARCH != O_RDONLY || (defined _WIN32 && !defined __CYGWIN__) +/* There is no point to forking if O_SEARCH conforms to POSIX, + or on native MS-Windows which lacks 'fork'. */ +bool try_fork = false; +# else +bool try_fork = errno == EACCES || errno == ESTALE; +# endif +if (!try_fork) { wd->state = ERROR_STATE; wd->val.errnum = errno; -- 2.43.0
Re: Possible UNINIT bug within man-db gl sources
On 2024-08-27 04:15, Marc Nieper-Wißkirchen wrote: However, see the second code block in Example 4 of section 6.7.3.1 (in the latest draft of C23). This may, of course, be itself an error as the example is about restrict and not about uninitialized structs. Yes, that example sheds no light on the use of uninitialized variables. It's obvious that it's only about "restrict".
Re: tty heuristics in lib/readutmp.c exclude "web" sessions
On 2024-08-26 01:51, Allison Karlitskaya wrote: So I guess my wish here is open-ended, but something like "please allow for a way to ensure that web logins appear in the output of who" If we're simply trying to emulate the old readutmp it sounds like this should be the default behavior.
Re: [PATCH] diffseq: port to clang 18.1.6 in ‘patch’
On 2024-08-26 00:17, Sam James wrote: It would happen on mandriva where they patch their Clang to identify as newer GCC. It can also happen in Fedora or Ubuntu if you configure with CC='clang -fgnuc-version=14.2.1'. Not clear whether we need to support that sort of thing.
Re: [PATCH] diffseq: port to clang 18.1.6 in ‘patch’
On 2024-08-24 09:05, Bruno Haible wrote: So, when we have a conditional that tests for GCC >= 4.7 or 10 or 14, we don't need to exclude clang, because it's already excluded. Weird. I can no longer reproduce the problem with clang. So I undid that change. If the problem happens again I'll be more careful about documenting the environment.
[PATCH] diffseq: port to clang 18.1.6 in ‘patch’
* lib/diffseq.h: Omit the pragmas if __clang__. --- ChangeLog | 5 + lib/diffseq.h | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c9e76d63d..29c02f5478 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2024-08-24 Paul Eggert + + diffseq: port to clang 18.1.6 in ‘patch’ + * lib/diffseq.h: Omit the pragmas if __clang__. + 2024-08-24 Bruno Haible relocatable-prog: Add support for 32-bit build on FreeBSD/powerpc64. diff --git a/lib/diffseq.h b/lib/diffseq.h index 362df177af..5f1f173363 100644 --- a/lib/diffseq.h +++ b/lib/diffseq.h @@ -95,7 +95,7 @@ /* Suppress gcc's "...may be used before initialized" warnings, generated by GCC versions up to at least GCC 14.2. Likewise for gcc -fanalyzer's "use of uninitialized value" warnings. */ -#if 4 <= __GNUC__ + (7 <= __GNUC_MINOR__) +#if 4 <= __GNUC__ + (7 <= __GNUC_MINOR__) && !__clang__ # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wmaybe-uninitialized" # if 13 <= __GNUC__ @@ -558,7 +558,7 @@ compareseq (OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET ylim, #undef XREF_YREF_EQUAL } -#if __GNUC__ + (__GNUC_MINOR__ >= 7) > 4 +#if 4 <= __GNUC__ + (7 <= __GNUC_MINOR__) && !__clang__ # pragma GCC diagnostic pop #endif -- 2.46.0
[PATCH] diffseq: port to GCC 14.2.1 in ‘patch’
* lib/diffseq.h: Also suppress -Wanalyzer-use-of-uninitialized-value. This fixes an unwanted diagnostic when compiling GNU ‘patch’ with gcc (GCC) 14.2.1 20240801 (Red Hat 14.2.1-1) x86-64. --- ChangeLog | 7 +++ lib/diffseq.h | 8 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5e0e0c013a..00db30f078 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2024-08-23 Paul Eggert + + diffseq: port to GCC 14.2.1 in ‘patch’ + * lib/diffseq.h: Also suppress -Wanalyzer-use-of-uninitialized-value. + This fixes an unwanted diagnostic when compiling GNU ‘patch’ + with gcc (GCC) 14.2.1 20240801 (Red Hat 14.2.1-1) x86-64. + 2024-08-23 Bruno Haible host-cpu-c-abi: Update comment, working around FreeBSD breakage. diff --git a/lib/diffseq.h b/lib/diffseq.h index 0c5bc9cbc6..362df177af 100644 --- a/lib/diffseq.h +++ b/lib/diffseq.h @@ -93,10 +93,14 @@ #endif /* Suppress gcc's "...may be used before initialized" warnings, - generated by GCC versions up to at least GCC 13.2. */ -#if __GNUC__ + (__GNUC_MINOR__ >= 7) > 4 + generated by GCC versions up to at least GCC 14.2. + Likewise for gcc -fanalyzer's "use of uninitialized value" warnings. */ +#if 4 <= __GNUC__ + (7 <= __GNUC_MINOR__) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +# if 13 <= __GNUC__ +# pragma GCC diagnostic ignored "-Wanalyzer-use-of-uninitialized-value" +# endif #endif /* -- 2.46.0
Re: coreutils 9.5 problem cross-compiling using uClibc-ng
On 2024-08-21 14:25, Waldemar Brodkorb wrote: -#if defined __GLIBC__ && 2 < __GLIBC__ + (2 <= __GLIBC_MINOR__) +#if defined __GLIBC__ && 2 < __GLIBC__ + (2 <= __GLIBC_MINOR__) && !defined __UCLIBC__ Thanks, I installed the attached into Gnulib.From 9765bc796b3e6ceaa7a10ba07c9c2f1e272a4249 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 21 Aug 2024 23:00:38 -0700 Subject: [PATCH] mcel: port to uClibc-ng MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Waldemar Brodkorb in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00130.html * lib/mcel.h (mcel_scan): Don’t treat uClibc-ng like glibc. --- ChangeLog | 7 +++ lib/mcel.h | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 5fdae94326..0bf44b8e45 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2024-08-21 Paul Eggert + + mcel: port to uClibc-ng + Problem reported by Waldemar Brodkorb in: + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00130.html + * lib/mcel.h (mcel_scan): Don’t treat uClibc-ng like glibc. + 2024-08-21 Bruno Haible stdio: Fix compilation error in C++ mode on Linux/riscv64 with musl. diff --git a/lib/mcel.h b/lib/mcel.h index 7d92d24601..d9f8385155 100644 --- a/lib/mcel.h +++ b/lib/mcel.h @@ -226,7 +226,8 @@ mcel_scan (char const *p, char const *lim) /* An initial mbstate_t; initialization optimized for some platforms. For details about these and other platforms, see wchar.in.h. */ -#if defined __GLIBC__ && 2 < __GLIBC__ + (2 <= __GLIBC_MINOR__) +#if (defined __GLIBC__ && 2 < __GLIBC__ + (2 <= __GLIBC_MINOR__) \ + && !defined __UCLIBC__) /* Although only a trivial optimization, it's worth it for GNU. */ mbstate_t mbs; mbs.__count = 0; #elif (defined __FreeBSD__ || defined __DragonFly__ || defined __OpenBSD__ \ -- 2.43.0
Re: Possible UNINIT bug within man-db gl sources
On 2024-08-21 06:36, Bruno Haible wrote: In summary, this paragraph makes no sense for structs. Hmm, well, the paragraph makes sense to me. I suppose somebody could fire off a question to the C committee about the intent of the paragraph. In the meantime it's an engineering decision - should we port to verrry picky implementations that complain about components of the struct not being initialized? This is a valid question regardless of the standard's intent. I expect that it's not high priority to change the code, if the only picky implementation is SAST (which I've never used and which I suspect is chock-full of false positives anyway). If it's GCC's -fanalyzer or sanitizer or valgrind or something else we (or at least I) commonly use, though, that'd be a different matter. Also, if the C committee comes back and says behavior is undefined, we might want to make the change, if only as insurance. that 'return (struct ...) { initializers }' produces particularly good code with modern compilers) Yeah, that's partly why I weighed in on this seemingly-obscure topic. I like the more-functional style and wouldn't want "undefined behavior" to get in its way.
Re: Support for leftmost-shortest matches in EREs in POSIX-2024
On 2024-08-19 16:36, Ed Morton wrote: so - what are the plans, if any, for supporting that functionality? I don't know of any plans. It'd be nontrivial to add the functionality. We'd like to have it of course.
Re: [PATCH 2/2] error: merge from glibc and with verror
On 2024-08-17 07:54, Bruno Haible wrote: Hi Paul, [2]: https://git.savannah.gnu.org/cgit/m4.git/commit/?h=branch-1.4&id=b357b798b04053df437b2df2f4f42dca69fb764c Thanks. But now, in m4, there is a "make distcheck" failure: ... An update of po/POTFILES.in is in order, I think. Like you did in coreutils. Thanks for reporting that. Should be fixed now.
Re: Possible UNINIT bug within man-db gl sources
On 2024-08-16 07:30, Bruno Haible wrote: Copying and then discarding an uninitialized value of integer or pointer type (i.e. not a signalling NaN (*)) is not undefined behaviour. Although that's how typical implementations behave, the C standard says that copying from a source variable has undefined behavior if the source could have been declared with 'register' (i.e., it's auto and never has its address taken). See C23 §6.3.2.1 paragraph 2. Since the code in question is copying from such a source, its behavior is technically undefined. I recall running into a similar problem myself, when writing the mcel code, though I don't recall the details. Proposed patch attached. I daresay there's lots of other places that would need a similar fix, assuming this one is acceptable.From 442c99cf9db881ddaa591ae829c4a37f14e014ee Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 16 Aug 2024 22:07:18 -0700 Subject: [PATCH] avltree-list: avoid undefined behavior Problem reported by Lukas Javorsky in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00095.html * lib/gl_anytree_list2.h (gl_tree_iterator, gl_tree_iterator_from_to): Avoid undefined behavior in accessing uninitialized locals. --- ChangeLog | 8 lib/gl_anytree_list2.h | 38 ++ 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/ChangeLog b/ChangeLog index ee8851fea5..ce41dbf7e0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2024-08-16 Paul Eggert + + avltree-list: avoid undefined behavior + Problem reported by Lukas Javorsky in: + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00095.html + * lib/gl_anytree_list2.h (gl_tree_iterator, gl_tree_iterator_from_to): + Avoid undefined behavior in accessing uninitialized locals. + 2024-08-16 Bruno Haible gitsub.sh: For a submodule, merge from the right remote branch. diff --git a/lib/gl_anytree_list2.h b/lib/gl_anytree_list2.h index a66e611b14..d908bcec3c 100644 --- a/lib/gl_anytree_list2.h +++ b/lib/gl_anytree_list2.h @@ -581,26 +581,20 @@ gl_tree_list_free (gl_list_t list) static gl_list_iterator_t _GL_ATTRIBUTE_PURE gl_tree_iterator (gl_list_t list) { - gl_list_iterator_t result; gl_list_node_t node; - result.vtable = list->base.vtable; - result.list = list; /* Start node is the leftmost node. */ node = list->root; if (node != NULL) while (node->left != NULL) node = node->left; - result.p = node; - /* End point is past the rightmost node. */ - result.q = NULL; -#if defined GCC_LINT || defined lint - result.i = 0; - result.j = 0; - result.count = 0; -#endif - return result; + /* End point is past the rightmost node, so .q is a null pointer. */ + return (gl_list_iterator_t) { +.vtable = list->base.vtable, +.list = list, +.p = node, + }; } static gl_list_iterator_t _GL_ATTRIBUTE_PURE @@ -612,19 +606,15 @@ gl_tree_iterator_from_to (gl_list_t list, size_t start_index, size_t end_index) if (!(start_index <= end_index && end_index <= count)) /* Invalid arguments. */ abort (); - result.vtable = list->base.vtable; - result.list = list; - /* Start node is the node at position start_index. */ - result.p = (start_index < count ? node_at (list->root, start_index) : NULL); - /* End point is the node at position end_index. */ - result.q = (end_index < count ? node_at (list->root, end_index) : NULL); -#if defined GCC_LINT || defined lint - result.i = 0; - result.j = 0; - result.count = 0; -#endif - return result; + return (gl_list_iterator_t) { +.vtable = list->base.vtable, +.list = list, +/* Start node is the node at position start_index. */ +.p = start_index < count ? node_at (list->root, start_index) : NULL, +/* End point is the node at position end_index. */ +.q = end_index < count ? node_at (list->root, end_index) : NULL, + }; } static bool -- 2.43.0
Re: [PATCH 2/2] error: merge from glibc and with verror
On 2024-08-15 03:09, Bruno Haible wrote: Use attribute [[nodiscard]] wherever glibc uses __wur. Should we do the same thing, to mirror glibc attributes in gnulib's function declarations, for all other attributes? I'm inclined to think so, as it gives Gnulib and Gnulib-using packages heads-ups about changes coming down the pike from glibc.
Re: [PATCH 2/2] error: merge from glibc and with verror
Thanks, I installed the attached.From 9e60f2db903b17c1a31e24b89bda90a12446459d Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 15 Aug 2024 15:12:25 -0700 Subject: [PATCH] maint: adjust to recent removal of verror.c * po/POTFILES.in: Remove lib/verror.c --- po/POTFILES.in | 1 - 1 file changed, 1 deletion(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index d824499c5..a12436969 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -24,7 +24,6 @@ lib/siglist.h lib/strsignal.c lib/unicodeio.c lib/userspec.c -lib/verror.c lib/version-etc.c lib/xalloc-die.c lib/xbinary-io.c -- 2.43.0
Re: [PATCH 2/2] error: merge from glibc and with verror
On 2024-08-15 00:05, Bruno Haible wrote: Declaring Gnulib functions in general is just a matter of copying the same idiom that we have in hundreds of places in *.in.h files. Its history and genesis is not simple; but the result is simple to apply. I didn't find it simple in this case. For example, verror_with_line is treated quite differently from error_with_line, because I couldn't easily decide the right way to adapt error_with_line's decl. The situation is not quite the same, because error_with_line might replace and verror_with_line doesn't. Also, error_with_line doesn't have the problem that verror_with_line does with that funny macro business in error.c (see below). This is my usual experience, to be honest. I try to copy from existing decls but I usually get it wrong. It can be frustrating. I vaguely understand why everything is there (some of it to do with C++, unfortunately) but it's generally a tricky business. What I find to be an unusual complexity here is that verror and verror_at_line are defined by error.c; yet when I glance through error.c quickly I don't see verror and verror_at_line being defined there. Hiding function definitions behind macros... Yes, that complexity comes from the desire to keep error.c the same in Gnulib and glibc. Is it planned that glibc exports verror and verror_at_line at some point? I don't have plans in that direction. Would probably be a good idea if someone had the energy to do it. Also, since this was a breaking change, someone will need to send patches to coreutils, m4, man-db, patch, and libpipeline [1]. It is not nice if we leave it as a bad surprise to the maintainer of one of these packages. First I've heard about sending email. I thought that the NEWS entries were designed for that sort of thing? It's a judgment call as to whether Gnulib should keep around a one-line file lib/verror.h containing just "#include " for backward compatibility. Gnulib is a source-code package, where minor glitches are to be expected, so I figured we might as well go without that shim. To try this out I just now updated bleeding-edge coreutils and m4 to use the latest Gnulib[1][2]. Both needed to remove "#include " lines - the breakage you mentioned - but I found that a bigger hassle was the recently-added __attribute__ ((cold)), which wasn't my idea - that came from Glibc. Also m4 still has a bogus -Wnull-dereference diagnostic, unrelated to the verror.h changes, that somebody needs to investigate when they find the time. Looking at this slightly bigger picture, removing verror.h lines seems pretty minor. [1]: https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=827186453707186a5c18f163dcc0b7a4d63427c2 [2]: https://git.savannah.gnu.org/cgit/m4.git/commit/?h=branch-1.4&id=b357b798b04053df437b2df2f4f42dca69fb764c
Re: [PATCH 2/2] error: merge from glibc and with verror
Thanks, I attempted to fix that by installing the attached. It attempts to be simple rather than catering to fancy warnings even in the situation you mention. I wish I didn't have to remember the complicated rules for declaring Gnulib functions, and to some extent this simplistic patch is a perhaps-vain attempt to cut down on that complexity.From 48384fd1c637a2f683b634a4c3522eccc1b0dfe9 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 14 Aug 2024 20:04:42 -0700 Subject: [PATCH] verror: allow library name-spacing of verror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00085.html * lib/error.c (verror, verror_at_line) [!_LIBC]: #undef only if the corresponding GNULIB_defined_... macros are defined. * lib/error.in.h (verror, verror_with_line): Don't define as macros if they are already defined, or if _GL_NO_INLINE_ERROR is defined. This may lose some niceties with GCC warnings, but the code’s valid. --- lib/error.c| 8 ++-- lib/error.in.h | 18 ++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/error.c b/lib/error.c index 843e19304e..837d5e56c6 100644 --- a/lib/error.c +++ b/lib/error.c @@ -131,8 +131,12 @@ int strerror_r (int errnum, char *buf, size_t buflen); # define __strerror_r strerror_r # endif /* GNULIB_STRERROR_R_POSIX || HAVE_STRERROR_R || defined strerror_r */ -# undef verror -# undef verror_at_line +# if GNULIB_defined_verror +# undef verror +# endif +# if GNULIB_defined_verror_at_line +# undef verror_at_line +# endif #endif /* not _LIBC */ #if !_LIBC diff --git a/lib/error.in.h b/lib/error.in.h index 82022a9362..4804c8c465 100644 --- a/lib/error.in.h +++ b/lib/error.in.h @@ -213,8 +213,13 @@ extern void verror (int __status, int __errnum, const char *__format, va_list __args) _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_STANDARD, 3, 0)); -#define verror(status, ...) \ - __gl_error_call (verror, status, __VA_ARGS__) +#ifndef _GL_NO_INLINE_ERROR +# ifndef verror +# define verror(status, ...) \ + __gl_error_call (verror, status, __VA_ARGS__) +# define GNULIB_defined_verror 1 +# endif +#endif /* Print a message with 'vfprintf (stderr, FORMAT, ARGS)'; if ERRNUM is nonzero, follow it with ": " and strerror (ERRNUM). @@ -228,8 +233,13 @@ extern void verror_at_line (int __status, int __errnum, const char *__fname, va_list __args) _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_STANDARD, 5, 0)); -#define verror_at_line(status, ...) \ - __gl_error_call (verror_at_line, status, __VA_ARGS__) +#ifdef _GL_NO_INLINE_ERROR +# ifndef verror_at_line +# define verror_at_line(status, ...) \ + __gl_error_call (verror_at_line, status, __VA_ARGS__) +# define GNULIB_defined_verror_at_line 1 +# endif +#endif /* If NULL, error will flush stdout, then print on stderr the program name, a colon and a space. Otherwise, error will call this -- 2.43.0
[PATCH 2/2] error: merge from glibc and with verror
* lib/error.c: Merge changes since 2018 from glibc. - The following changes are taken from glibc: (__error_internal, __error_at_line_internal): New functions, with most of the old error and error_at_line_internal but with va_list and with a new trailing mode_args, for wide character mode. (error_tail): Add trailing arg. (error_tail, __error_internal, __error_at_line_internal): Redo _LIBC implementation with respect to cancelation. [_LIBC]: Include , not . - The following changes are specific to Gnulib: (__error_internal, __error_at_line_internal, error_tail) [!_LIBC]: Use macros to define away the new trailing arg, and to name the internal functions to verror and verror_at_line. (verror, verror_at_line) [!_LIBC]: Undef so that we omit the __gl_error_call business when defining these functions. * lib/error.in.h: Include stdarg.h. (verror, verror_at_line): New decls and macros. * m4/error_h.m4 ([gl_ERROR_H]): Always compile error.c if the verror module is also present. * modules/verror (Files, lib_SOURCES): Remove lib/verror.h, lib/verror.c. (Depends-on): Remove stdio, xvasprintf. (configure.ac-early): Define gl_HAVE_MODULE_VERROR so that the error module compiles error.c. Not sure if this is the standard way to do this, but it seems to work. * modules/verror (Include), tests/test-verror.c: Include error.h, not verror.h. --- ChangeLog | 30 NEWS| 3 + lib/error.c | 166 +--- lib/error.in.h | 31 + lib/verror.c| 80 - lib/verror.h| 62 - m4/error_h.m4 | 17 +++-- modules/verror | 10 ++- tests/test-verror.c | 2 +- 9 files changed, 143 insertions(+), 258 deletions(-) delete mode 100644 lib/verror.c delete mode 100644 lib/verror.h diff --git a/ChangeLog b/ChangeLog index bd5e7b1ce5..fe16aa1787 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,35 @@ 2024-08-14 Paul Eggert + error: merge from glibc and with verror + * lib/error.c: Merge changes since 2018 from glibc. + - The following changes are taken from glibc: + (__error_internal, __error_at_line_internal): New functions, + with most of the old error and error_at_line_internal but + with va_list and with a new trailing mode_args, + for wide character mode. + (error_tail): Add trailing arg. + (error_tail, __error_internal, __error_at_line_internal): + Redo _LIBC implementation with respect to cancelation. + [_LIBC]: Include , not . + - The following changes are specific to Gnulib: + (__error_internal, __error_at_line_internal, error_tail) [!_LIBC]: + Use macros to define away the new trailing arg, and to name + the internal functions to verror and verror_at_line. + (verror, verror_at_line) [!_LIBC]: Undef so that we omit + the __gl_error_call business when defining these functions. + * lib/error.in.h: Include stdarg.h. + (verror, verror_at_line): New decls and macros. + * m4/error_h.m4 ([gl_ERROR_H]): Always compile error.c if + the verror module is also present. + * modules/verror (Files, lib_SOURCES): + Remove lib/verror.h, lib/verror.c. + (Depends-on): Remove stdio, xvasprintf. + (configure.ac-early): Define gl_HAVE_MODULE_VERROR so that + the error module compiles error.c. Not sure if this is the + standard way to do this, but it seems to work. + * modules/verror (Include), tests/test-verror.c: + Include error.h, not verror.h. + error: it’s cold This mimics what glibc is doing nowadays. * lib/error.in.h (error, error_at_line): diff --git a/NEWS b/NEWS index 865b86ba3c..40a8da647a 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,9 @@ User visible incompatible changes DateModules Changes +2024-08-14 verror The include file is changed from "verror.h" +to . + 2024-08-09 full-read These modules now prefer signed types to size_t. full-write The preferred types are idx_t for object sizes and safe-read ptrdiff_t for values that are either a size or -1. diff --git a/lib/error.c b/lib/error.c index c53dfeb60d..843e19304e 100644 --- a/lib/error.c +++ b/lib/error.c @@ -1,25 +1,32 @@ /* Error handler for noninteractive utilities - Copyright (C) 1990-1998, 2000-2007, 2009-2024 Free Software Foundation, Inc. + Copyright (C) 1990-2024 Free Software Foundation, Inc. This file is part of the GNU C Library. - This file 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 free software; you can redistribute it and/or + modify it under the terms
[PATCH 1/2] error: it’s cold
This mimics what glibc is doing nowadays. * lib/error.in.h (error, error_at_line): * lib/verror.h (verror, verror_at_line): Declare with _GL_ATTRIBUTE_CODE. --- ChangeLog | 8 lib/error.in.h | 12 lib/verror.h | 4 +++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6e351bd70a..bd5e7b1ce5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2024-08-14 Paul Eggert + + error: it’s cold + This mimics what glibc is doing nowadays. + * lib/error.in.h (error, error_at_line): + * lib/verror.h (verror, verror_at_line): + Declare with _GL_ATTRIBUTE_CODE. + 2024-08-14 Bruno Haible pthread-rwlock-extra-tests: Exclude this test from packages by default. diff --git a/lib/error.in.h b/lib/error.in.h index 4deaf98ded..d2aede7d53 100644 --- a/lib/error.in.h +++ b/lib/error.in.h @@ -30,8 +30,8 @@ #ifndef _@GUARD_PREFIX@_ERROR_H #define _@GUARD_PREFIX@_ERROR_H -/* This file uses _GL_ATTRIBUTE_ALWAYS_INLINE, _GL_ATTRIBUTE_FORMAT, - _GL_ATTRIBUTE_MAYBE_UNUSED. */ +/* This file uses _GL_ATTRIBUTE_ALWAYS_INLINE, _GL_ATTRIBUTE_COLD, + _GL_ATTRIBUTE_FORMAT, _GL_ATTRIBUTE_MAYBE_UNUSED. */ #if !_GL_CONFIG_H_INCLUDED #error "Please include config.h first." #endif @@ -93,6 +93,7 @@ extern "C" { # endif _GL_FUNCDECL_RPL (error, void, (int __status, int __errnum, const char *__format, ...), + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 3, 4))); _GL_CXXALIAS_RPL (error, void, (int __status, int __errnum, const char *__format, ...)); @@ -105,6 +106,7 @@ _GL_CXXALIAS_RPL (error, void, # if ! @HAVE_ERROR@ _GL_FUNCDECL_SYS (error, void, (int __status, int __errnum, const char *__format, ...), + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 3, 4))); # endif _GL_CXXALIAS_SYS (error, void, @@ -117,7 +119,7 @@ _GL_CXXALIAS_SYS (error, void, #pragma GCC diagnostic ignored "-Wattributes" _GL_ATTRIBUTE_MAYBE_UNUSED static void -_GL_ATTRIBUTE_ALWAYS_INLINE +_GL_ATTRIBUTE_ALWAYS_INLINE _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 3, 4)) _gl_inline_error (int __status, int __errnum, const char *__format, ...) { @@ -148,6 +150,7 @@ _GL_CXXALIASWARN (error); _GL_FUNCDECL_RPL (error_at_line, void, (int __status, int __errnum, const char *__filename, unsigned int __lineno, const char *__format, ...), + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 5, 6))); _GL_CXXALIAS_RPL (error_at_line, void, (int __status, int __errnum, const char *__filename, @@ -162,6 +165,7 @@ _GL_CXXALIAS_RPL (error_at_line, void, _GL_FUNCDECL_SYS (error_at_line, void, (int __status, int __errnum, const char *__filename, unsigned int __lineno, const char *__format, ...), + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 5, 6))); # endif _GL_CXXALIAS_SYS (error_at_line, void, @@ -175,7 +179,7 @@ _GL_CXXALIAS_SYS (error_at_line, void, #pragma GCC diagnostic ignored "-Wattributes" _GL_ATTRIBUTE_MAYBE_UNUSED static void -_GL_ATTRIBUTE_ALWAYS_INLINE +_GL_ATTRIBUTE_ALWAYS_INLINE _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_ERROR, 5, 6)) _gl_inline_error_at_line (int __status, int __errnum, const char *__filename, unsigned int __lineno, const char *__format, ...) diff --git a/lib/verror.h b/lib/verror.h index deeec966fd..a951a5c3e1 100644 --- a/lib/verror.h +++ b/lib/verror.h @@ -17,7 +17,7 @@ #ifndef _VERROR_H #define _VERROR_H 1 -/* This file uses _GL_ATTRIBUTE_FORMAT. */ +/* This file uses _GL_ATTRIBUTE_COLD, _GL_ATTRIBUTE_FORMAT. */ #if !_GL_CONFIG_H_INCLUDED #error "Please include config.h first." #endif @@ -39,6 +39,7 @@ extern "C" { extern void verror (int __status, int __errnum, const char *__format, va_list __args) + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_STANDARD, 3, 0)); /* Print a message with 'vfprintf (stderr, FORMAT, ARGS)'; @@ -51,6 +52,7 @@ extern void verror (int __status, int __errnum, const char *__format, extern void verror_at_line (int __status, int __errnum, const char *__fname, unsigned int __lineno, const char *__format, va_list __args) + _GL_ATTRIBUTE_COLD _GL_ATTRIBUTE_FORMAT ((_GL_ATTRIBUTE_SPEC_PRINTF_STANDARD, 5, 0)); #ifdef __cplusplus -- 2.46.0
Re: [PATCH] test-utime: port to noatime file systems
On 2024-08-10 17:57, Collin Funk wrote: All seem to have Y2K: Yes, and that's the correct value. Thanks to both you and Bruno for reporting it. I installed the attached.From 8fa1fba5dab1a126c25e534867f653359039d177 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 10 Aug 2024 21:47:07 -0700 Subject: [PATCH] test-fdutimensat: fix newly-added test Problem reported by Bruno Haible and Collin Funk in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00061.html https://lists.gnu.org/r/bug-gnulib/2024-08/msg00064.html * tests/test-utimens.h (test_utimens): Fix typo in new test, which causes the test to break on file systems that are not noatime. --- ChangeLog| 9 + tests/test-utimens.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 5938c70ea9..0f232e39db 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2024-08-10 Paul Eggert + + test-fdutimensat: fix newly-added test + Problem reported by Bruno Haible and Collin Funk in: + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00061.html + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00064.html + * tests/test-utimens.h (test_utimens): Fix typo in new test, + which causes the test to break on file systems that are not noatime. + 2024-08-10 Collin Funk htonl: Add tests. diff --git a/tests/test-utimens.h b/tests/test-utimens.h index 256d48de90..575507f735 100644 --- a/tests/test-utimens.h +++ b/tests/test-utimens.h @@ -194,7 +194,7 @@ test_utimens (int (*func) (char const *, struct timespec const *), bool print) ASSERT (stat (BASE "link", &st2) == 0); if (check_atime) { -ASSERT (st2.st_atime == BILLION); +ASSERT (st2.st_atime == Y2K); ASSERT (get_stat_atime_ns (&st2) == 0); } ASSERT (st2.st_mtime == Y2K); -- 2.43.0
Re: [PATCH] full-read, etc.: prefer signed types
On 2024-08-09 14:56, Bruno Haible wrote: But why also for full-read and full-write? These have a calling convention like fread() and fwrite() — a result < count is an error —, and fread() and fwrite() return size_t. It's for the same reason I'm gradually switching to idx_t elsewhere. In well-controlled apps object sizes cannot exceed PTRDIFF_MAX so there's no generality loss in replacing size_t with idx_t. And there's a win in switching, because idx_t arithmetic is less error-prone.
[PATCH] full-read, etc.: prefer signed types
* lib/full-read.h, lib/full-write.h, lib/safe-read.h, lib/safe-write.h: Include idx.h. * lib/full-write.c (full_read, full_write): Now accept and returns idx_t. * lib/safe-read.c (bufptr): New type, since apps are not supposed to #define keywords like ‘const’. (safe_read, safe_write): Now accept idx_t and return ptrdiff_t. * lib/safe-read.h (SAFE_READ_ERROR): * lib/safe-write.h (SAFE_WRITE_ERROR): Now ptrdiff_t, not size_t. * modules/full-read, modules/full-write, modules/safe-read: * modules/safe-write (Depends-on): Add idx. --- ChangeLog | 16 NEWS | 6 ++ lib/full-read.h| 4 +++- lib/full-write.c | 10 +- lib/full-write.h | 3 ++- lib/safe-read.c| 14 +++--- lib/safe-read.h| 11 +++ lib/safe-write.h | 11 +++ modules/full-read | 1 + modules/full-write | 1 + modules/safe-read | 1 + modules/safe-write | 1 + 12 files changed, 57 insertions(+), 22 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7c9f9bce76..ff3704336b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2024-08-09 Paul Eggert + + full-read, etc.: prefer signed types + * lib/full-read.h, lib/full-write.h, lib/safe-read.h, lib/safe-write.h: + Include idx.h. + * lib/full-write.c (full_read, full_write): + Now accept and returns idx_t. + * lib/safe-read.c (bufptr): New type, since apps are not + supposed to #define keywords like ‘const’. + (safe_read, safe_write): Now accept idx_t and return ptrdiff_t. + * lib/safe-read.h (SAFE_READ_ERROR): + * lib/safe-write.h (SAFE_WRITE_ERROR): + Now ptrdiff_t, not size_t. + * modules/full-read, modules/full-write, modules/safe-read: + * modules/safe-write (Depends-on): Add idx. + 2024-08-09 Bruno Haible sig2str: Align with POSIX:2024. diff --git a/NEWS b/NEWS index 4c5d9fc5de..865b86ba3c 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,12 @@ User visible incompatible changes DateModules Changes +2024-08-09 full-read These modules now prefer signed types to size_t. +full-write The preferred types are idx_t for object sizes and +safe-read ptrdiff_t for values that are either a size or -1. +safe-write SAFE_READ_ERROR and SAFE_WRITE_ERROR are now +obsolescent; callers can just check for < 0. + 2024-06-22 xvasprintf It is now the programmer's responsibility to pass c-xvasprintfa valid format string without %ls, %lc directives and that all widths are >= -INT_MAX and <= INT_MAX. diff --git a/lib/full-read.h b/lib/full-read.h index 617702a60b..9d44e4fbc9 100644 --- a/lib/full-read.h +++ b/lib/full-read.h @@ -17,6 +17,8 @@ #include +#include "idx.h" + #ifdef __cplusplus extern "C" { #endif @@ -25,7 +27,7 @@ extern "C" { /* Read COUNT bytes at BUF to descriptor FD, retrying if interrupted or if partial reads occur. Return the number of bytes successfully read, setting errno if that is less than COUNT. errno = 0 means EOF. */ -extern size_t full_read (int fd, void *buf, size_t count); +extern idx_t full_read (int fd, void *buf, idx_t count); #ifdef __cplusplus diff --git a/lib/full-write.c b/lib/full-write.c index 8e27b9c134..0559a0a128 100644 --- a/lib/full-write.c +++ b/lib/full-write.c @@ -54,16 +54,16 @@ When writing, set errno if fewer than COUNT bytes are written. When reading, if fewer than COUNT bytes are read, you must examine errno to distinguish failure from EOF (errno == 0). */ -size_t -full_rw (int fd, const void *buf, size_t count) +idx_t +full_rw (int fd, const void *buf, idx_t count) { - size_t total = 0; + idx_t total = 0; const char *ptr = (const char *) buf; while (count > 0) { - size_t n_rw = safe_rw (fd, ptr, count); - if (n_rw == (size_t) -1) + ptrdiff_t n_rw = safe_rw (fd, ptr, count); + if (n_rw < 0) break; if (n_rw == 0) { diff --git a/lib/full-write.h b/lib/full-write.h index 87f9b928b9..7265499618 100644 --- a/lib/full-write.h +++ b/lib/full-write.h @@ -17,6 +17,7 @@ #include +#include "idx.h" #ifdef __cplusplus extern "C" { @@ -26,7 +27,7 @@ extern "C" { /* Write COUNT bytes at BUF to descriptor FD, retrying if interrupted or if partial writes occur. Return the number of bytes successfully written, setting errno if that is less than COUNT. */ -extern size_t full_write (int fd, const void *buf, size_t count); +extern idx_t full_write (int fd, const void *buf, idx_t count); #ifdef __cplusplus diff --git a/lib/safe-read.c b/lib/safe-read.c index a389b57bfb..c7d31d535d 100644 --- a/lib/safe-read.c +++ b/lib/safe-read.c @@ -42,22 +42,22 @@ #ifdef SAFE_WRITE # define safe_rw safe_write # define rw write +typedef void const *bufpt
[PATCH] test-utime: port to noatime file systems
Problem encountered on Ubuntu 24.04 zfs mounted noatime. * tests/test-fdutimensat.c (main): * tests/test-futimens.h (test_futimens): * tests/test-lutimens.h (test_lutimens): * tests/test-utime.c (test_utime): * tests/test-utimens-common.h (checkable_atime): New function. * tests/test-utimens.h (test_utimens): * tests/test-utimensat.c (main): Do not check atime on file systems mounted noatime. --- ChangeLog | 13 doc/posix-functions/futimens.texi | 4 +++ doc/posix-functions/utimensat.texi | 4 +++ doc/posix-functions/utimes.texi| 4 +++ tests/test-fdutimensat.c | 10 -- tests/test-futimens.h | 36 ++ tests/test-lutimens.h | 49 +++--- tests/test-utime.c | 27 +++- tests/test-utimens-common.h| 21 + tests/test-utimens.h | 46 +--- tests/test-utimensat.c | 12 ++-- 11 files changed, 171 insertions(+), 55 deletions(-) diff --git a/ChangeLog b/ChangeLog index c31e68d8ee..e82dd02834 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2024-08-09 Paul Eggert + + test-utime: port to noatime file systems + Problem encountered on Ubuntu 24.04 zfs mounted noatime. + * tests/test-fdutimensat.c (main): + * tests/test-futimens.h (test_futimens): + * tests/test-lutimens.h (test_lutimens): + * tests/test-utime.c (test_utime): + * tests/test-utimens-common.h (checkable_atime): New function. + * tests/test-utimens.h (test_utimens): + * tests/test-utimensat.c (main): + Do not check atime on file systems mounted noatime. + 2024-08-09 Bruno Haible pthread-rwlock: Attempt to avoid test failure on some machines. diff --git a/doc/posix-functions/futimens.texi b/doc/posix-functions/futimens.texi index 2c8beca267..7df36c09ce 100644 --- a/doc/posix-functions/futimens.texi +++ b/doc/posix-functions/futimens.texi @@ -35,6 +35,10 @@ glibc 2.11, musl libc, Solaris 11. Portability problems not fixed by Gnulib: @itemize @item +On file systems mounted with the @code{noatime} attribute, +this function might not modify the access time as requested: +Linux kernel 6.9. +@item Some platforms lack the ability to change the timestamps of a file descriptor, so the replacement can fail with @code{ENOSYS}; the gnulib module @samp{utimens} provides a more reliable interface @code{fdutimens}. diff --git a/doc/posix-functions/utimensat.texi b/doc/posix-functions/utimensat.texi index 9024235416..189c5cb79a 100644 --- a/doc/posix-functions/utimensat.texi +++ b/doc/posix-functions/utimensat.texi @@ -44,6 +44,10 @@ AIX 7.2. Portability problems not fixed by Gnulib: @itemize @item +On file systems mounted with the @code{noatime} attribute, +this function might not modify the access time as requested: +Linux kernel 6.9. +@item On some platforms, timestamps of symbolic links cannot be modified, so the replacement fails with @code{ENOSYS} if passed the flag @code{AT_SYMLINK_NOFOLLOW} on a symlink. diff --git a/doc/posix-functions/utimes.texi b/doc/posix-functions/utimes.texi index 70ab62f4b7..98a97184d9 100644 --- a/doc/posix-functions/utimes.texi +++ b/doc/posix-functions/utimes.texi @@ -24,6 +24,10 @@ some platforms incorrectly round rather than truncate. Use @code{utimensat(AT_FDCWD,file,times,0)}, or the gnulib module @code{utimens}, instead. @item +On file systems mounted with the @code{noatime} attribute, +this function might not modify the access time as requested: +Linux kernel 6.9. +@item On some platforms, @code{utimes (file, NULL)} fails to set the file's timestamp to the current time: glibc 2.3.3. diff --git a/tests/test-fdutimensat.c b/tests/test-fdutimensat.c index a21e297f03..5f8d0f6d07 100644 --- a/tests/test-fdutimensat.c +++ b/tests/test-fdutimensat.c @@ -118,7 +118,7 @@ main (void) /* Directory relative tests. */ ASSERT (mkdir (BASE "dir", 0700) == 0); ASSERT (chdir (BASE "dir") == 0); - fd = creat ("file", 0600); + fd = open ("file", O_RDWR | O_CREAT | O_TRUNC, 0600); ASSERT (0 <= fd); errno = 0; ASSERT (fdutimensat (AT_FDCWD, fd, ".", NULL, 0) == -1); @@ -126,13 +126,17 @@ main (void) { struct timespec ts[2]; struct stat st; +bool check_atime = checkable_atime (fd, &st); ts[0].tv_sec = Y2K; ts[0].tv_nsec = 0; ts[1] = ts[0]; ASSERT (fdutimensat (fd, dfd, BASE "dir/file", ts, 0) == 0); ASSERT (stat ("file", &st) == 0); -ASSERT (st.st_atime == Y2K); -ASSERT (get_stat_atime_ns (&st) == 0); +if (check_atime) + { +ASSERT (st.st_atime == Y2K); +ASSERT (get_stat_atime_ns (&st) == 0); + } ASSERT (st.st_mtime == Y2K); ASSERT (get_stat_mtime_ns (&st) == 0); } diff --git a/tests/test-futimens.h b/tests/test-futi
Re: [PATCH] errno: make EEXIST != ENOTEMPTY on AIX
On 2024-08-07 02:58, Bruno Haible wrote: Did it possibly embody the assumption that errno values are small integers or all near together? Yes it did. My preference for static checking went overboard, it seems. I installed the attached, which I hope fixes it.From 37fcd9c9d822150f807211de0fec6c7d86de60ef Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 7 Aug 2024 07:23:51 -0700 Subject: [PATCH] errno-tests: port to GNU/Hurd Test for errno distinctness dynamically rather than statically, since the static test blows up the compiler on Hurd. Problem reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2024-08/msg00039.html Also, test that errno values can all be used in #if, and improve diagnostics. * tests/test-errno.c: Include stdio.h, stdlib.h, string.h. (USABLE_IN_IF): New macro. Use it to check errno values in #if. (ERRTAB): New macro. (struct nameval): New type. (errtab): New global variable. (errtab_cmp): New function. (main): Test for errno distinctness dynamically not statically. Diagnose lack of distinctness better. --- ChangeLog | 18 +++ tests/test-errno.c | 57 +++--- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8e0896f127..df710ddfa6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2024-08-07 Paul Eggert + + errno-tests: port to GNU/Hurd + Test for errno distinctness dynamically rather than statically, + since the static test blows up the compiler on Hurd. + Problem reported by Bruno Haible in: + https://lists.gnu.org/r/bug-gnulib/2024-08/msg00039.html + Also, test that errno values can all be used in #if, + and improve diagnostics. + * tests/test-errno.c: Include stdio.h, stdlib.h, string.h. + (USABLE_IN_IF): New macro. Use it to check errno values in #if. + (ERRTAB): New macro. + (struct nameval): New type. + (errtab): New global variable. + (errtab_cmp): New function. + (main): Test for errno distinctness dynamically not statically. + Diagnose lack of distinctness better. + 2024-08-07 Bruno Haible fchmodat: Fix cross-compilation guess. diff --git a/tests/test-errno.c b/tests/test-errno.c index 7403ee1432..acd07ab95b 100644 --- a/tests/test-errno.c +++ b/tests/test-errno.c @@ -20,6 +20,10 @@ #include +#include +#include +#include + /* Check all POSIX-defined errno values, using M (v) to check value v. */ #define CHECK_POSIX_ERRNOS(m) \ m (E2BIG) \ @@ -107,24 +111,51 @@ #define POSITIVE_INTEGER_CONSTANT_EXPRESSION(e) static_assert (0 < (e) << 0); CHECK_POSIX_ERRNOS (POSITIVE_INTEGER_CONSTANT_EXPRESSION) +/* Verify that errno values can all be used in #if. */ +#define USABLE_IN_IF(e) ^ e +#if 0 CHECK_POSIX_ERRNOS (USABLE_IN_IF) +#endif + +/* Check that errno values all differ, except possibly for + EWOULDBLOCK == EAGAIN and ENOTSUP == EOPNOTSUPP. */ +#define ERRTAB(e) { #e, e }, +static struct nameval { char const *name; int value; } + errtab[] = { CHECK_POSIX_ERRNOS (ERRTAB) }; + +static int +errtab_cmp (void const *va, void const *vb) +{ + struct nameval const *a = va, *b = vb; + + /* Sort by value first, then by name (to simplify later tests). + Subtraction cannot overflow as both are positive. */ + int diff = a->value - b->value; + return diff ? diff : strcmp (a->name, b->name); +} + int main () { + int test_exit_status = EXIT_SUCCESS; + /* Verify that errno can be assigned. */ errno = EOVERFLOW; /* Check that errno values all differ, except possibly for - EWOULDBLOCK == EAGAIN and ENOTSUP == EOPNOTSUPP. */ - #define INDEXED_BY_ERRNO(e) [e] = 1, - #define ERRNO_COUNT(e) 0, - static char const -indexed_by_errno[] = { CHECK_POSIX_ERRNOS (INDEXED_BY_ERRNO) }, -errno_count[] = { CHECK_POSIX_ERRNOS (ERRNO_COUNT) }; - int distinct_errnos = 0; - for (int i = 0; i < sizeof indexed_by_errno; i++) -distinct_errnos += indexed_by_errno[i]; - return ((sizeof errno_count - - (EWOULDBLOCK == EAGAIN) - - (ENOTSUP == EOPNOTSUPP)) - != distinct_errnos); + EAGAIN == EWOULDBLOCK and ENOTSUP == EOPNOTSUPP. */ + int nerrtab = sizeof errtab / sizeof *errtab; + qsort (errtab, nerrtab, sizeof *errtab, errtab_cmp); + for (int i = 1; i < nerrtab; i++) +if (errtab[i - 1].value == errtab[i].value) + { +fprintf (stderr, "%s == %s == %d\n", + errtab[i - 1].name, errtab[i].name, errtab[i].value); +if (! ((strcmp ("EAGAIN", errtab[i - 1].name) == 0 +&& strcmp ("EWOULDBLOCK", errtab[i].name) == 0) + || (strcmp ("ENOTSUP", errtab[i - 1].name) == 0 + && strcmp ("EOPNOTSUPP", errtab[i].name) == 0))) + test_exit_status = EXIT_FAILURE; + } + + return test_exit_status; } -- 2.43.0
Re: date without time
On 2024-08-02 01:42, Bruno Haible wrote: A more intelligent programmer will answer it with "A day is a period of 23 or 24 or 25 hours, going from 00:00 to 24:00 of each date" The TZDB timezones don't always fit that paradigm exactly. In Samoa, the "day" 1892-07-04 was a period of 48 hours, because they moved the clock back 24 hours at midnight so that visiting Americans could celebrate an double-length Fourth of July. This has been called "a masterpiece of diplomatic flattery". Something similar happened in Kwajalein, where 1969-09-30 lasted 47 hours. Conversely, 1993-08-21 lasted zero hours in Kwajalein, as they advanced the clock 24 hours at midnight. The TZDB timezone data handle both the Samoa and the Kwajalein cases, and glibc supports that.
Re: [PATCH] errno: make EEXIST != ENOTEMPTY on AIX
On 2024-08-01 05:08, Bruno Haible wrote: It is better to define such a macro in the "early" phase of configure, like we do for the 'extensions' module. Thanks for fixing that. I had originally thought to put the "#define _LINUX_SOURCE_COMPAT 1" into AC_USE_SYSTEM_EXTENSIONS, which would have fixed it in a different way. However, it's not really an extension to AIX but more of a change (since it changes the strerror_r API) so I didn't do that. With so many Gnulib modules now defining _LINUX_SOURCE_COMPAT directly or indirectly, we're probably not that far from what my original thought would have done, in a practical sense. But it's clearer to have a separate macro since it really is a behavior change, not an extension.
Re: parse_datetime() and RFC 9557 suffixes
The whole business of time zone offsets is a mess. First, it's not clear whether ISO 8601 requires ":" in numeric time zone offsets. RFC 3339 Appendix A suggests it's not. Second, in practice the colon often is omitted; it's certainly omitted by POSIX and ISO C strftime %z, by the Time Zone Database, and by lots of other places - this is partly due to the influence of RFC 5322 and its predecessors and related standards, all of which omit the colon. The Time Zone Database and many other sources also commonly used offsets that contain just hours, e.g., "+02", which I think conforms to ISO 8601 (hard to tell, since ISO 8601 is not freely readable). Third, RFC 9557 does not allow either "2023-09-20[+02:00]" or "2023-09-20[+0200]". It requires a time before the timezone. (This issue is minor; I've already mentioned this to Alejandro, so he knows what he's proposing is not standardized by ISO or by the RFC.) Fourth, RFC 9557 section 1.2 strongly discourages suffixes like "[+02:00]". That suffix means clocks never change in that location, which is historically inaccurate everywhere. Instead, you're supposed to use a suffix like "[Africa/Tripoli]". I view this as a major objection to Alejandro's proposal for the output of "passwd". It might make sense to extend parse-datetime (and GNU date -d) to parse RFC 9557 [time-zone] suffixes, should they become popular. That should be doable. It might also be nice to extend strftime (and therefore GNU date +FMT) to generate them, though that'd be a bit harder since the info is not always easily available. However, none of this should be needed for the 'passwd' command. For that, it's probably better just to stick with its current output, which is just a date without time zone.
[PATCH] errno: make EEXIST != ENOTEMPTY on AIX
Also, improve errno tests. * m4/calloc.m4 (gl_FUNC_CALLOC_GNU): * m4/malloc.m4 (gl_FUNC_MALLOC_GNU): * m4/realloc.m4 (gl_FUNC_REALLOC_GNU): * m4/scandir.m4 (gl_FUNC_SCANDIR): Define _LINUX_SOURCE_COMPAT, as this can sometimes help on AIX. * m4/errno_h.m4 (gl_HEADER_ERRNO_H): Define _LINUX_SOURCE_COMPAT, to make EEXIST != ENOTEMPTY. * m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Define _LINUX_SOURCE_COMPAT, in case someone else does. * modules/errno-tests (Depends-on): Add assert-h, c99. * tests/test-errno.c (e1, ..., e131): Remove, replacing with ... (CHECK_POSIX_ERRNOS, POSITIVE_INTEGER_CONSTANT_EXPRESSION) (INDEXED_BY_ERRNO, ERRNO_COUNT): These new macros. Check that all errno values are positive integer constant expressions. Check that they are all distinct, except perhaps for EWOULDBLOCK == EAGAIN and ENOTSUP == EOPNOTSUPP. Also check ESOCKTNOSUPPORT, added in POSIX.1-2024. Also, check that errno values are distinct except when POSIX says they needn’t be distinct, since POSIX.1-2024 gives license to GNU/Linux’s non-distinct values. --- ChangeLog | 25 doc/posix-functions/calloc.texi | 2 +- doc/posix-functions/malloc.texi | 2 +- doc/posix-functions/realloc.texi| 2 +- doc/posix-functions/strerror_r.texi | 3 +- doc/posix-headers/errno.texi| 4 + lib/errno.in.h | 2 +- m4/calloc.m4| 7 +- m4/errno_h.m4 | 7 +- m4/malloc.m4| 7 +- m4/passfd.m4| 4 +- m4/realloc.m4 | 7 +- m4/scandir.m4 | 5 +- m4/strerror_r.m4| 9 +- modules/errno-tests | 3 +- tests/test-errno.c | 187 +++- 16 files changed, 174 insertions(+), 102 deletions(-) diff --git a/ChangeLog b/ChangeLog index bdedb82c3a..78e4bd2f09 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2024-07-31 Paul Eggert + + errno: make EEXIST != ENOTEMPTY on AIX + Also, improve errno tests. + * m4/calloc.m4 (gl_FUNC_CALLOC_GNU): + * m4/malloc.m4 (gl_FUNC_MALLOC_GNU): + * m4/realloc.m4 (gl_FUNC_REALLOC_GNU): + * m4/scandir.m4 (gl_FUNC_SCANDIR): + Define _LINUX_SOURCE_COMPAT, as this can sometimes help on AIX. + * m4/errno_h.m4 (gl_HEADER_ERRNO_H): + Define _LINUX_SOURCE_COMPAT, to make EEXIST != ENOTEMPTY. + * m4/strerror_r.m4 (gl_FUNC_STRERROR_R): + Define _LINUX_SOURCE_COMPAT, in case someone else does. + * modules/errno-tests (Depends-on): Add assert-h, c99. + * tests/test-errno.c (e1, ..., e131): Remove, replacing with ... + (CHECK_POSIX_ERRNOS, POSITIVE_INTEGER_CONSTANT_EXPRESSION) + (INDEXED_BY_ERRNO, ERRNO_COUNT): These new macros. + Check that all errno values are positive integer constant expressions. + Check that they are all distinct, except perhaps for + EWOULDBLOCK == EAGAIN and ENOTSUP == EOPNOTSUPP. + Also check ESOCKTNOSUPPORT, added in POSIX.1-2024. + Also, check that errno values are distinct except when POSIX says + they needn’t be distinct, since POSIX.1-2024 gives license to + GNU/Linux’s non-distinct values. + 2024-07-31 Bruno Haible float: Update to mostly guarantee ISO C 23 compliance. diff --git a/doc/posix-functions/calloc.texi b/doc/posix-functions/calloc.texi index 20fb026a2f..9e3b5a4929 100644 --- a/doc/posix-functions/calloc.texi +++ b/doc/posix-functions/calloc.texi @@ -30,5 +30,5 @@ It fixes this portability problem: @item @code{calloc (0, s)} and @code{calloc (n, 0)} return @code{NULL} on success on some platforms: -AIX 7.2. +AIX 7.3. @end itemize diff --git a/doc/posix-functions/malloc.texi b/doc/posix-functions/malloc.texi index 2137ba2b81..9ec84fce5d 100644 --- a/doc/posix-functions/malloc.texi +++ b/doc/posix-functions/malloc.texi @@ -27,5 +27,5 @@ by fixing this portability problem: @itemize @item @code{malloc (0)} returns @code{NULL} on success on some platforms: -AIX 7.2. +AIX 7.3. @end itemize diff --git a/doc/posix-functions/realloc.texi b/doc/posix-functions/realloc.texi index 6bb61dd7e4..bd130195c2 100644 --- a/doc/posix-functions/realloc.texi +++ b/doc/posix-functions/realloc.texi @@ -40,7 +40,7 @@ It fixes these portability problems: @itemize @item @code{realloc (NULL, 0)} returns @code{NULL} on success on some platforms: -AIX 7.2. +AIX 7.3. @item On some platforms, @code{realloc (p, 0)} with non-null @code{p} diff --git a/doc/posix-functions/strerror_r.texi b/doc/posix-functions/strerror_r.texi index cff950f963..52f56fcfc5 100644 --- a/doc/posix-functions/strerror_r.texi +++ b/doc/posix-functions/strerror_r.texi @@ -20,7 +20,8 @@ Portability problems fixed by Gnulib: This function is missing on some platforms: NetBSD 3.0, Minix 3.1.8, HP-UX 11.23, Solaris 9, mingw, MSVC 14. @item -glibc, Cygwin, and Android have an incompatible version
Re: should asprintf and friends guarantee sane pointer on failure?
On 2024-07-29 07:35, Bruno Haible wrote: What can happen with the result pointer ptr if asprintf (&ptr, ...) fails? There are four possibilities: (a) ptr is unchanged. (b) ptr is set to NULL. (c) ptr is set to a non-NULL pointer that is not free()able. (d) ptr is set to a non-NULL pointer that is freshly allocated and thus meant to be freed. It is quite obvious that no reasonable implementation will do (c) nor (d). (c) because that would be an invitation for doing free(invalid_pointer). Actually (c) might be better, as that would be more likely to catch bugs in programs. That is, asprintf could set ptr to ((void *) 42) so that the resulting mistaken 'free' traps. This might be helpful, at least as a debugging option. However, that's lower priority. More important: As usual, we can have a *-gnu variant of the modules. The question is whether such a change in behaviour is worth all the implementation effort. I'm not quite following this. We already have vasprintf-gnu, and since the glibc behavior is changing to set ptr to NULL it shouldn't be much implementation effort to add that to the vasprintf-gnu module. Similarly for Gnulib's own modules like c-vaszprintf-gnu. What am I missing here?
Re: [PATCH] timevar: add missing timevar.def
On 2024-07-28 12:07, Bruno Haible wrote: What was the immediate problem that you had with this module? I was testing all modules that depend on c99, this way: ./gnulib-tool -h --create-testdir --dir foo $(cd modules && grep -lx 'c99' $(git ls-files)) cd foo ./configure make The last command fails as follows on Fedora 40 x86-64 when building Gnullib commit 56a8b46608f60d33cc4026875e8a52b1bf8a2e16: ... gcc -DHAVE_CONFIG_H -I. -I.. -DGNULIB_STRICT_CHECKING=1 -g -O2 -MT timevar.o -MD -MP -MF $depbase.Tpo -c -o timevar.o timevar.c && mv -f $depbase.Tpo $depbase.Po In file included from timevar.c:24: timevar.h:80:10: fatal error: timevar.def: No such file or directory 80 | #include "timevar.def" | ^ compilation terminated. make[4]: *** [Makefile:4705: timevar.o] Error 1 make[4]: Leaving directory '/home/eggert/src/gnu/gnulib/foo/gllib' make[3]: *** [Makefile:4751: all-recursive] Error 1 make[3]: Leaving directory '/home/eggert/src/gnu/gnulib/foo/gllib' make[2]: *** [Makefile:3935: all] Error 2 make[2]: Leaving directory '/home/eggert/src/gnu/gnulib/foo/gllib' make[1]: *** [Makefile:2484: all-recursive] Error 1 make[1]: Leaving directory '/home/eggert/src/gnu/gnulib/foo'
[PATCH 2/2] maint: remove ‘coding: utf-8’ on ASCII
These days, ‘coding: utf-8’ tags aren’t needed in general, as Emacs typically defaults to UTF-8, and Emacs itself now has many untagged source files in UTF-8. However, for now let’s just remove the tags when they are on ASCII files. * lib/md5-stream.c, lib/sha1-stream.c, lib/sha256-stream.c: * lib/sm3-stream.c: Remove unnecessary coding: lines. --- ChangeLog | 8 lib/md5-stream.c| 7 --- lib/sha1-stream.c | 7 --- lib/sha256-stream.c | 7 --- lib/sm3-stream.c| 7 --- 5 files changed, 8 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index eb7df14aea..51337ddca6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2024-07-28 Paul Eggert + maint: remove ‘coding: utf-8’ on ASCII + These days, ‘coding: utf-8’ tags aren’t needed in general, + as Emacs typically defaults to UTF-8, and Emacs itself now + has many untagged source files in UTF-8. However, for now let’s + just remove the tags when they are on ASCII files. + * lib/md5-stream.c, lib/sha1-stream.c, lib/sha256-stream.c: + * lib/sm3-stream.c: Remove unnecessary coding: lines. + maint: remove coding: that did not work * m4/fnmatch.m4, m4/fpieee.m4, m4/mbrlen.m4, m4/mbrtowc.m4: Remove ineffective coding: strings. diff --git a/lib/md5-stream.c b/lib/md5-stream.c index c82f18145e..fdd2bd8b4b 100644 --- a/lib/md5-stream.c +++ b/lib/md5-stream.c @@ -132,10 +132,3 @@ process_partial_block: free (buffer); return 0; } - -/* - * Hey Emacs! - * Local Variables: - * coding: utf-8 - * End: - */ diff --git a/lib/sha1-stream.c b/lib/sha1-stream.c index 7bf44e5ea3..cbdf95ab76 100644 --- a/lib/sha1-stream.c +++ b/lib/sha1-stream.c @@ -120,10 +120,3 @@ sha1_stream (FILE *stream, void *resblock) free (buffer); return 0; } - -/* - * Hey Emacs! - * Local Variables: - * coding: utf-8 - * End: - */ diff --git a/lib/sha256-stream.c b/lib/sha256-stream.c index 08d24b7bfc..690ca967dd 100644 --- a/lib/sha256-stream.c +++ b/lib/sha256-stream.c @@ -136,10 +136,3 @@ sha224_stream (FILE *stream, void *resblock) return shaxxx_stream (stream, "sha224", resblock, SHA224_DIGEST_SIZE, sha224_init_ctx, sha224_finish_ctx); } - -/* - * Hey Emacs! - * Local Variables: - * coding: utf-8 - * End: - */ diff --git a/lib/sm3-stream.c b/lib/sm3-stream.c index c534edb126..f9ed05a944 100644 --- a/lib/sm3-stream.c +++ b/lib/sm3-stream.c @@ -114,10 +114,3 @@ sm3_stream (FILE *stream, void *resblock) free (buffer); return 0; } - -/* - * Hey Emacs! - * Local Variables: - * coding: utf-8 - * End: - */ -- 2.45.2
[PATCH 1/2] maint: remove coding: that did not work
* m4/fnmatch.m4, m4/fpieee.m4, m4/mbrlen.m4, m4/mbrtowc.m4: Remove ineffective coding: strings. --- ChangeLog | 4 m4/fnmatch.m4 | 2 +- m4/fpieee.m4 | 2 +- m4/mbrlen.m4 | 2 +- m4/mbrtowc.m4 | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 00a963d179..eb7df14aea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2024-07-28 Paul Eggert + maint: remove coding: that did not work + * m4/fnmatch.m4, m4/fpieee.m4, m4/mbrlen.m4, m4/mbrtowc.m4: + Remove ineffective coding: strings. + std-gnu23: new module The plan is to update the c99 module to depend on std-gnu23 instead of on std-gnu11, and to make std-gnu11 obsolete. diff --git a/m4/fnmatch.m4 b/m4/fnmatch.m4 index 6e45b8aa00..5a9e85976b 100644 --- a/m4/fnmatch.m4 +++ b/m4/fnmatch.m4 @@ -1,5 +1,5 @@ # fnmatch.m4 -# serial 21 -*- coding: utf-8 -*- +# serial 21 dnl Copyright (C) 2000-2007, 2009-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, diff --git a/m4/fpieee.m4 b/m4/fpieee.m4 index 665543d0ed..086d51ddf5 100644 --- a/m4/fpieee.m4 +++ b/m4/fpieee.m4 @@ -1,5 +1,5 @@ # fpieee.m4 -# serial 2 -*- coding: utf-8 -*- +# serial 2 dnl Copyright (C) 2007, 2009-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, diff --git a/m4/mbrlen.m4 b/m4/mbrlen.m4 index fd37fe98fb..f7fad22d67 100644 --- a/m4/mbrlen.m4 +++ b/m4/mbrlen.m4 @@ -1,5 +1,5 @@ # mbrlen.m4 -# serial 12 -*- coding: utf-8 -*- +# serial 12 dnl Copyright (C) 2008, 2010-2024 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, diff --git a/m4/mbrtowc.m4 b/m4/mbrtowc.m4 index 62c4fdb348..4ea8184049 100644 --- a/m4/mbrtowc.m4 +++ b/m4/mbrtowc.m4 @@ -1,5 +1,5 @@ # mbrtowc.m4 -# serial 44 -*- coding: utf-8 -*- +# serial 44 dnl Copyright (C) 2001-2002, 2004-2005, 2008-2024 Free Software Foundation, dnl Inc. dnl This file is free software; the Free Software Foundation -- 2.45.2
[PATCH] std-gnu23: new module
The plan is to update the c99 module to depend on std-gnu23 instead of on std-gnu11, and to make std-gnu11 obsolete. A benefit of this will be that c99 will no longer affect C++. For now, though, std-gnu23 is simply a new, optional module. * MODULES.html.sh (func_all_modules): Add it. * modules/std-gnu11: Change description. * modules/std-gnu23, m4/std-gnu23.m4: New files. * m4/std-gnu11.m4: Do nothing if _AC_C_C23_OPTIONS is defined. (AC_PROG_CC): Use AC_DEFUN, not AC_DEFUN_ONCE, so that we can be superseded. --- ChangeLog | 11 + MODULES.html.sh | 1 + m4/std-gnu11.m4 | 8 +- m4/std-gnu23.m4 | 737 ++ modules/std-gnu11 | 2 +- modules/std-gnu23 | 19 ++ 6 files changed, 775 insertions(+), 3 deletions(-) create mode 100644 m4/std-gnu23.m4 create mode 100644 modules/std-gnu23 diff --git a/ChangeLog b/ChangeLog index 764de7d2fb..00a963d179 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2024-07-28 Paul Eggert + std-gnu23: new module + The plan is to update the c99 module to depend on std-gnu23 + instead of on std-gnu11, and to make std-gnu11 obsolete. + A benefit of this will be that c99 will no longer affect C++. + For now, though, std-gnu23 is simply a new, optional module. + * MODULES.html.sh (func_all_modules): Add it. + * modules/std-gnu23, m4/std-gnu23.m4: New files. + * m4/std-gnu11.m4: Do nothing if _AC_C_C23_OPTIONS is defined. + (AC_PROG_CC): Use AC_DEFUN, not AC_DEFUN_ONCE, so that + we can be superseded. + timevar: add missing timevar.def * modules/timevar (Files): Add timevar.def. diff --git a/MODULES.html.sh b/MODULES.html.sh index 4df6497fc3..b3990e070a 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2457,6 +2457,7 @@ func_all_modules () func_begin_table func_module alignasof func_module nullptr + func_module std-gnu23 func_module stdckdint func_end_table diff --git a/m4/std-gnu11.m4 b/m4/std-gnu11.m4 index 37324c158e..e8d5de7a1e 100644 --- a/m4/std-gnu11.m4 +++ b/m4/std-gnu11.m4 @@ -1,8 +1,11 @@ # std-gnu11.m4 -# serial 1 +# serial 2 # Prefer GNU C11 and C++11 to earlier versions. -*- coding: utf-8 -*- +# The std-gnu23 module, which defines _AC_C_C23_OPTIONS, supersedes us. +m4_ifndef([_AC_C_C23_OPTIONS], [ + # This implementation is taken from GNU Autoconf lib/autoconf/c.m4 # commit 017d5ddd82854911f0119691d91ea8a1438824d6 # dated Sun Apr 3 13:57:17 2016 -0700 @@ -38,7 +41,7 @@ m4_version_prereq([2.70], [], [ # COMPILER ... is a space separated list of C compilers to search for. # This just gives the user an opportunity to specify an alternative # search list for the C compiler. -AC_DEFUN_ONCE([AC_PROG_CC], +AC_DEFUN([AC_PROG_CC], [AC_LANG_PUSH(C)dnl AC_ARG_VAR([CC], [C compiler command])dnl AC_ARG_VAR([CFLAGS], [C compiler flags])dnl @@ -830,3 +833,4 @@ dnl with extended modes being tried first. ])# m4_version_prereq +])# !_AC_C_C23_OPTIONS diff --git a/m4/std-gnu23.m4 b/m4/std-gnu23.m4 new file mode 100644 index 00..db65c2bc36 --- /dev/null +++ b/m4/std-gnu23.m4 @@ -0,0 +1,737 @@ +# std-gnu23.m4 +# serial 1 + +# Prefer GNU C23 to earlier versions. + +# This implementation is taken from GNU Autoconf lib/autoconf/c.m4 +# commit 653956f44683e1daed9827de429590bdd2e317e7 +# dated Tue Apr 30 10:26:50 2024 -0700 +# This implementation will be obsolete once we can assume Autoconf 2.73 +# or later is installed everywhere a Gnulib program might be developed. + +m4_version_prereq([2.73], [], [ + + +# Copyright (C) 2001-2024 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/>. + +# Written by David MacKenzie, with help from +# Akim Demaille, Paul Eggert, +# François Pinard, Karl Berry, Richard Pixley, Ian Lance Taylor, +# Roland McGrath, Noah Friedman, david d zuhn, and many others. + + +# AC_PROG_CC([COMPILER ...]) +# -- +# COMPILER ... is a space separated list of C compilers to search for. +# This just gives the user an opportunity to specify an alternative +# search list for the C compiler. +AC_DEFUN([AC_PROG_CC], +[AC_LANG_PUSH(C)dnl +AC_ARG_VAR([CC], [C compiler command])dnl +AC_ARG_VAR([CFLAGS], [C compiler flags])dnl +_AC_ARG_VAR_LDFLAGS()dnl +_AC_ARG_VAR_LIBS()dnl +_AC_ARG_VAR_CPPFLAGS()dnl +m4_ifval([$1], + [AC_CHECK_TO
[PATCH] timevar: add missing timevar.def
* modules/timevar (Files): Add timevar.def. --- ChangeLog | 5 + modules/timevar | 1 + 2 files changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 2b0b7425bc..764de7d2fb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2024-07-28 Paul Eggert + + timevar: add missing timevar.def + * modules/timevar (Files): Add timevar.def. + 2024-07-27 Collin Funk doc: Remove HTML code tags from Texinfo files. diff --git a/modules/timevar b/modules/timevar index ef86fedb29..a460a08de8 100644 --- a/modules/timevar +++ b/modules/timevar @@ -4,6 +4,7 @@ A simple self-profiling module based on timers. Files: lib/timevar.h lib/timevar.c +lib/timevar.def Depends-on: c99 -- 2.45.2
[PATCH] strftime: improve doc
* lib/strftime.h: In comment, use active voice, reword errno description for clarity, and omit reference to draft POSIX 202x to simplify future maintenance. --- lib/strftime.h | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/strftime.h b/lib/strftime.h index 8ce62cdb6d..3179874af9 100644 --- a/lib/strftime.h +++ b/lib/strftime.h @@ -21,11 +21,11 @@ extern "C" { #endif -/* Formats the broken-down time *__TP, with additional __NS nanoseconds, +/* Format the broken-down time *__TP, with additional __NS nanoseconds, into the buffer __S of size __MAXSIZE, according to the rules of the LC_TIME category of the current locale. - Uses the time zone __TZ. + Use the time zone __TZ. If *__TP represents local time, __TZ should be set to tzalloc (getenv ("TZ")). If *__TP represents universal time (a.k.a. GMT), __TZ should be set to @@ -60,15 +60,11 @@ extern "C" { time zone: %z %Z nanosecond %N - Stores the result, as a string with a trailing NUL character, at the - beginning of the array __S[0..__MAXSIZE-1], if it fits, and returns - the length of that string, not counting the trailing NUL. In this case, - errno is preserved if the return value is 0. - If it does not fit, this function sets errno to ERANGE and returns 0. - Upon other errors, this function sets errno and returns 0 as well. - - Note: The errno behavior is in draft POSIX 202x plus some requested - changes to POSIX. + Store the result, as a string with a trailing NUL character, at the + beginning of the array __S[0..__MAXSIZE-1] and return the length of + that string, not counting the trailing NUL, and without changing errno. + If unsuccessful, possibly change the array __S, set errno, and return 0; + errno == ERANGE means the string didn't fit. This function is like strftime, but with two more arguments: * __TZ instead of the local timezone information, -- 2.45.2
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-25 14:27, Bruno Haible wrote: I have a hard time understanding the point and purpose of what you are saying: - In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00171.html> you removed the validation of 'base', saying: "strtol can support base 1 as an extension, xstrtol now does whatever the system does". - In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00238.html> you agreed to leaving it unspecified whether xstrtol sets *endptr in some case. Fair enough, I suppose I added too much gingerbread there, so I installed the attached patch to go back to the old way of doing things, as that is simpler, less confusing, and a bit more efficient. The idea is to not worry about hypothetical implementations or applications, just real ones. The only thing missing from the old way of doing things, was documentation that you shouldn't call xstrtol with a base other than 0 or 2 through 36, so the attached patch adds this to xstrtol.h. documentation like "If you pass a base other than 0 or 2..36, you can't expect a particular parsing, nor a particular error code, nor a particular value for *endptr." is good. I'm afraid we'll have to disagree on this particular case. It's OK for behavior to be undefined when the base is bad, just as it's OK for it to be undefined if the string is bad (e.g., when nptr == (char *) &errno). There's no real-world hazard, and this email thread has taken more of our time than the issue is worth. I didn't adjust the recently-added Gnulib test cases even though they're testing what is now documented to be undefined behavior, as they still pass on the FreeBSD and Ubuntu platforms that I tried them on. If they start failing we can remove them as needed. They're not testing anything important, after all.From bd1e981434c98751b1106a1744e77a27317b52b3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 25 Jul 2024 17:24:20 -0700 Subject: [PATCH] xstrtol: remove the base-checking changes * lib/xstrtol.c (__xstrtol): Stop worrying about hypothetical implementations that are causing more confusion than the code is worth. Instead, go back more to old way of doing things. None of this matters for practical applications. * lib/xstrtol.h: Document that behavior is undefined if the base is negative, 1, or greater than 36. * modules/xstrtol (Depends-on): Remove nullptr; no longer needed. --- ChangeLog | 11 +++ lib/xstrtol.c | 9 +++-- lib/xstrtol.h | 2 ++ modules/xstrtol | 1 - 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 21b1840dec..0c35378606 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2024-07-25 Paul Eggert + + xstrtol: remove the base-checking changes + * lib/xstrtol.c (__xstrtol): Stop worrying about hypothetical + implementations that are causing more confusion than the code is + worth. Instead, go back more to old way of doing things. + None of this matters for practical applications. + * lib/xstrtol.h: Document that behavior is undefined + if the base is negative, 1, or greater than 36. + * modules/xstrtol (Depends-on): Remove nullptr; no longer needed. + 2024-07-25 Bruno Haible xstrtol, xstrtoll tests: Avoid test failure on FreeBSD. diff --git a/lib/xstrtol.c b/lib/xstrtol.c index 797d3e4dee..4e7c0fcfda 100644 --- a/lib/xstrtol.c +++ b/lib/xstrtol.c @@ -71,7 +71,7 @@ strtol_error __xstrtol (char const *nptr, char **endptr, int base, __strtol_t *val, char const *valid_suffixes) { - char *t_ptr = nullptr; + char *t_ptr; char **p = endptr ? endptr : &t_ptr; if (! TYPE_SIGNED (__strtol_t)) @@ -88,13 +88,10 @@ __xstrtol (char const *nptr, char **endptr, int base, } errno = 0; - __strtol_t tmp = __strtol (nptr, &t_ptr, base); - if (!t_ptr) -return LONGINT_INVALID; - *p = t_ptr; + __strtol_t tmp = __strtol (nptr, p, base); strtol_error err = LONGINT_OK; - if (*p == nptr && (errno == 0 || errno == EINVAL)) + if (*p == nptr) { /* If there is no number but there is a valid suffix, assume the number is 1. The string is invalid otherwise. */ diff --git a/lib/xstrtol.h b/lib/xstrtol.h index 1fab748810..45c8921578 100644 --- a/lib/xstrtol.h +++ b/lib/xstrtol.h @@ -47,6 +47,8 @@ typedef enum strtol_error strtol_error; - Return strtol_error, and store any result through an additional TYPE *VAL pointer instead of returning the result. - If TYPE is unsigned, reject leading '-'. + - Behavior is undefined if BASE is negative, 1, or greater than 36. + (In this respect xstrtol acts like the C standard, not like POSIX.) - Accept an additional char const *VALID_SUFFIXES pointer to a possibly-empty string containing allowed numeric suffixes, which multiply the value. These include SI suffixes like &
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-25 13:11, Bruno Haible wrote: +strtol_error s_err = __xstrtol (input, &endp, -1, &val, "k"); +ASSERT (s_err == LONGINT_INVALID); +ASSERT (endp == NULL); +ASSERT (val == -17); This isn't a valid test case. If the base is -1, the underlying strtol can support any base notation it likes. It might, for example, support base 64 and the "k" would be valid input. If the documentation doesn't make it sufficiently clear that the behavior is not fully specified with an invalid base, we can do that. But let's not waste time testing corner cases that don't matter.
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-25 09:39, Alejandro Colomar wrote: So, we have arrived there, tmp is 1, and the suffix is "k", so it will be multiplied by 1000. That's the expected behavior. I passed a base of -1. Fine, so don't do that. When you use base -1, it doesn't matter whether the behavior differs on platform A vs B. Portable code can't use base -1 with strtol; we'll get unspecified or even undefined behavior. It's OK if xstrtol also has unspecified or even undefined behavior when the base is -1. Again, none of this matters in actual software that uses xstrtol. Let's move on to a different topic.
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-25 02:27, Alejandro Colomar wrote: - tmp = 1; We will multiply the base by 1, even if it wasn't in the string. I don't see that. There's no multiplication of BASE by 1 in "tmp = 1;". I don't understand too much the rest of xstrtol(), but I think it will successfully parse 1000, when it should have failed. There's no reason xstrtol should fail in that situation. Again, this is just a tempest in a teapot; real code doesn't run into this situation.
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-24 14:07, Alejandro Colomar wrote: I forgot to reply to the last part: "Why isn't that [current gnulib] good enough? With an implementation of strtol(3) that does what I wrote above, the test `if (!t_ptr)` isn't true, so we go to `if (*p == nptr && (errno == 0 || errno == EINVAL))`, which is true. Assuming that the string matches a valid suffix, it'd succeed, but the call should have failed early. No, because that code is dealing with the case where the number's text is missing. And if the number's text is missing it doesn't matter what the base is; there are no digits to multiply the base by. I see that this part of the API isn't documented; it should be. I installed the attached to try to fix that. > (And the worst part might be suggesting> readers that an invalid base can be successfully tested after a call to strtol(3), so that they do something similar.) That's OK; xstrtol.c need not warn about all the problems of strtol. Our readers' time is limited and this issue isn't important enough to consume their time. (In practice, as I wrote earlier, the base is always valid.)From 2ebf179bf3660b80f2391e69003f9572bb3e52fb Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 24 Jul 2024 17:00:13 -0700 Subject: [PATCH] xstrtol: improve xstrtol.h comment --- lib/xstrtol.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/xstrtol.h b/lib/xstrtol.h index 10b48cac4e..1fab748810 100644 --- a/lib/xstrtol.h +++ b/lib/xstrtol.h @@ -54,7 +54,9 @@ typedef enum strtol_error strtol_error; includes '0' they can be followed by "B" to stand for the usual SI powers of 1000 (or by "iB" to stand for powers of 1024 as before). Other supported suffixes include 'K' for 1024 or 1000, 'b' for 512, - 'c' for 1, and 'w' for 2. */ + 'c' for 1, and 'w' for 2. + - Suppose that after the initial whitespace, the number is missing + but there is a valid suffix. Then the number is treated as 1. */ #define _DECLARE_XSTRTOL(name, type) \ strtol_error name (char const *restrict /*nptr*/, \ -- 2.43.0
Re: [PATCH] lib/xstrtol.c: Fix xstrtol() on EINVAL (invalid base)
On 2024-07-24 12:27, Alejandro Colomar wrote: POSIX leaves the value of *endptr unspecified if the base is invalid; it is not unmodified. Fine, we can let xstrtol do the same. That, combined with the fact that POSIX allows implementations of strtol(3) to set EINVAL when no digits are found, makes it impossible to portably check if the base was valid after the call. I don't see why not. Current gnulib xstrtol.c does something reasonable if the base is invalid. Why isn't that good enough? - char *t_ptr = nullptr; + char *t_ptr; char **p = endptr ? endptr : &t_ptr; + *p = (char *) nptr; + + if (base < 0 || base == 1 || 36 < base) +return LONGINT_INVALID; + This would cause xstrtol to fail even if the underlying strtol supports base 1, base 64, etc. Why would we want to do that? I'm tired of strtol(3). :) Likewise. This issue about an invalid base is unimportant in practice, as the base is almost invariably a valid constant.
Re: xstrtod, xstrtold: proposed change
On 2024-07-23 12:35, Bruno Haible wrote: I propose to change the gradual underflow behaviour of xstrtod Sounds good. The current behavior with MSVC does make more sense.
Re: XMALLOC() et al
On 2024-07-20 03:27, Bruno Haible wrote: This is where our opionions differ. Yes, and I'm well aware of the advantages you listed for XMALLOC. In practice, for me, they don't outweigh the disadvantages. (In that sense they're like the advantages that C++ has over C)
Re: doc: structure of function substitutes chapter
On 2024-07-20 06:41, Bruno Haible wrote: I'm thinking that the same structure should also be applied to chapter 10. Makes sense to me too, thanks. That's what the C standard does.
Re: XMALLOC() et al
On 2024-07-19 18:59, Bruno Haible wrote: GNU gettext uses the XMALLOC macro in more than 100 places. It's just so convenient to do a memory allocation in 1 line of code: I find it more convenient to write this: context = xmalloc (sizeof *context); than this (taken from GNU gettext): context = XMALLOC (markup_parse_context_ty); not simply because it's easier for a human to read and know instantly that it's right, but because when one changes the type of 'context' one needn't change all corresponding xmalloc calls. Although XMALLOC has some advantages, all in all in my experience they're outweighed by its disadvantages. I would rather not encourage its use in Gnulib proper. I know that in some places, such as readutmp.c, you like to optimize several memory allocations into a single one. Yes, and that flexibility is another advantage of xmalloc etc. over XMALLOC etc.
Re: XMALLOC() et al
On 2024-07-19 17:13, Alejandro Colomar wrote: Are XMALLOC et al. part of the public API, or are they only internal helper macros? Are there current users of it (apart from gnulib itself)? I'm interested in discussing some details about that set of macros if they're not set in stone. They're not documented anywhere other than the source code. They're a bit controversial in the sense that I would rather not see their use proliferate. In a sense they're a poor substitute for C++ templates and my feeling is that if you want C++ you should use C++.
Re: [PATCH v1 0/2] xstrtol() fixes
Thanks, I installed the attached, which should fix the problems you mentioned in a less-invasive way.From 16b33e6649425fcdce095f262da98b539d2f7448 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 19 Jul 2024 10:39:58 -0700 Subject: [PATCH] xstrtol: be more robust against odd failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/xstrtol.c (__xstrtol): Don’t update *endptr if strtol doesn’t. Also, if the underlying strtol gives an unusual error number and sets *endpnr = nptr, assume that’s an error not a missing number. Problems reported by Alejandro Colomar in: https://lists.gnu.org/r/bug-gnulib/2024-07/msg00175.html https://lists.gnu.org/r/bug-gnulib/2024-07/msg00176.html * modules/xstrtol (Depends-on): Add nullptr. --- ChangeLog | 11 +++ lib/xstrtol.c | 15 ++- modules/xstrtol | 1 + 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 072a28855d..4e88d446a1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2024-07-19 Paul Eggert + + xstrtol: be more robust against odd failures + * lib/xstrtol.c (__xstrtol): Don’t update *endptr if strtol doesn’t. + Also, if the underlying strtol gives an unusual error number and + sets *endpnr = nptr, assume that’s an error not a missing number. + Problems reported by Alejandro Colomar in: + https://lists.gnu.org/r/bug-gnulib/2024-07/msg00175.html + https://lists.gnu.org/r/bug-gnulib/2024-07/msg00176.html + * modules/xstrtol (Depends-on): Add nullptr. + 2024-07-19 Bruno Haible doc: Mention function that were added in ISO C23. diff --git a/lib/xstrtol.c b/lib/xstrtol.c index c3145171f3..797d3e4dee 100644 --- a/lib/xstrtol.c +++ b/lib/xstrtol.c @@ -71,9 +71,8 @@ strtol_error __xstrtol (char const *nptr, char **endptr, int base, __strtol_t *val, char const *valid_suffixes) { - char *t_ptr; + char *t_ptr = nullptr; char **p = endptr ? endptr : &t_ptr; - *p = (char *) nptr; if (! TYPE_SIGNED (__strtol_t)) { @@ -82,14 +81,20 @@ __xstrtol (char const *nptr, char **endptr, int base, while (isspace (ch)) ch = *++q; if (ch == '-') -return LONGINT_INVALID; +{ + *p = (char *) nptr; + return LONGINT_INVALID; +} } errno = 0; - __strtol_t tmp = __strtol (nptr, p, base); + __strtol_t tmp = __strtol (nptr, &t_ptr, base); + if (!t_ptr) +return LONGINT_INVALID; + *p = t_ptr; strtol_error err = LONGINT_OK; - if (*p == nptr) + if (*p == nptr && (errno == 0 || errno == EINVAL)) { /* If there is no number but there is a valid suffix, assume the number is 1. The string is invalid otherwise. */ diff --git a/modules/xstrtol b/modules/xstrtol index ef49dfc357..1f8a28fa6c 100644 --- a/modules/xstrtol +++ b/modules/xstrtol @@ -9,6 +9,7 @@ m4/xstrtol.m4 Depends-on: intprops +nullptr stdckdint stdint -- 2.43.0
Re: [PATCH v1] xstrtol: 1 is not a valid base
On 2024-07-18 12:53, Alejandro Colomar wrote: I think it'd be common to assume that unless specifically documented, you behave like POSIX's strtol(3), which produces defined behavior for a base of 1. Fair enough. I installed the attached Gnulib patch which addresses that point, along with the other points you raised about xstrtol that I understood. Since the system strtol can support base 1 as an extension, xstrtol now does whatever the system does so there's no need for the 'assume'. (Not that any Gnulib-using programs care) (Oh, and by the way, I don't think I put that 'assume'/'assert'/whatever code in there decades ago as I'm not a fan of that sort of thing) PS. Sorry about missing a space in the ChangeLog entry - I fixed that in a followup patch.From 64ddc975e72cb55d2b2d755c25603bd70312aa5e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 18 Jul 2024 17:28:36 -0700 Subject: [PATCH] xstrtol: document and stray less from strtol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch alters xstrtol behavior slightly, in areas are not likely to affect any real callers, by making xstrtol behave more like the system strtol. In particular, it lets xstrtol support bases other than those required by POSIX, if the underlying implementation does; this removes the need for an ‘assert’. It also uses ‘restrict’ like the underlying strtol does. The patch also documents xstrtol in xstrtol.h. Problems reported by AlejandroColomar in: https://lists.gnu.org/r/bug-gnulib/2024-07/msg00159.html * lib/xstrtol.c: Do not include stdio.h or assure.h. (__xstrtol): Use same parameter names as POSIX, to make it easier for outsiders to follow. Assume C99 decls after statement. Do not require the base to be 0, or 2-36, as the underlying implementation is allowed to support other bases. Do not assume isspace preserves errno. * lib/xstrtol.h (_DECLARE_XSTRTOL): Declare pointers to be ‘restrict’, for compatibility with C. * m4/xstrtol.m4 (gl_XSTRTOL): Require AC_C_RESTRICT. * modules/xstrtol (Depends-on): Remove ‘assure’. --- ChangeLog | 23 +++ lib/xstrtol.c | 59 +++-- lib/xstrtol.h | 17 +- m4/xstrtol.m4 | 3 ++- modules/xstrtol | 1 - 5 files changed, 64 insertions(+), 39 deletions(-) diff --git a/ChangeLog b/ChangeLog index 833ecd6d80..1728846392 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2024-07-18 Paul Eggert + + xstrtol: document and stray less from strtol + This patch alters xstrtol behavior slightly, in areas are not + likely to affect any real callers, by making xstrtol behave more + like the system strtol. In particular, it lets xstrtol support + bases other than those required by POSIX, if the underlying + implementation does; this removes the need for an ‘assert’. + It also uses ‘restrict’ like the underlying strtol does. + The patch also documents xstrtol in xstrtol.h. + Problems reported by AlejandroColomar in: + https://lists.gnu.org/r/bug-gnulib/2024-07/msg00159.html + * lib/xstrtol.c: Do not include stdio.h or assure.h. + (__xstrtol): Use same parameter names as POSIX, to make it + easier for outsiders to follow. Assume C99 decls after statement. + Do not require the base to be 0, or 2-36, as the underlying + implementation is allowed to support other bases. + Do not assume isspace preserves errno. + * lib/xstrtol.h (_DECLARE_XSTRTOL): Declare pointers to be + ‘restrict’, for compatibility with C. + * m4/xstrtol.m4 (gl_XSTRTOL): Require AC_C_RESTRICT. + * modules/xstrtol (Depends-on): Remove ‘assure’. + 2024-07-18 Bruno Haible abort-debug: Don't assume that signal SIGABRT is unmasked and unhandled. diff --git a/lib/xstrtol.c b/lib/xstrtol.c index 575c16d45f..c3145171f3 100644 --- a/lib/xstrtol.c +++ b/lib/xstrtol.c @@ -30,10 +30,6 @@ #include "xstrtol.h" -/* Some pre-ANSI implementations (e.g. SunOS 4) - need stderr defined if assertion checking is enabled. */ -#include - #include #include #include @@ -45,7 +41,6 @@ # include #endif -#include "assure.h" #include "intprops.h" static strtol_error @@ -72,26 +67,17 @@ bkm_scale_by_power (__strtol_t *x, int base, int power) return err; } -/* FIXME: comment. */ - strtol_error -__xstrtol (const char *s, char **ptr, int strtol_base, - __strtol_t *val, const char *valid_suffixes) +__xstrtol (char const *nptr, char **endptr, int base, + __strtol_t *val, char const *valid_suffixes) { char *t_ptr; - char **p; - __strtol_t tmp; - strtol_error err = LONGINT_OK; - - assure (0 == strtol_base || (2 <= strtol_base && strtol_base <= 36)); - - p = (ptr ? ptr : &t_ptr); - - errno = 0; + char **p = endptr ? endptr : &t_ptr; + *p = (char *) nptr; if (! TYPE_SIGNED (__strtol_t)) { - const char *q = s; + char
Re: stack-trace: Use libasan as an alternative to libbacktrace
On 2024-07-18 04:21, Bruno Haible wrote: on NetBSD 10.0, every program that links with libasan exits without even reaching main(). It merely prints "This sanitizer is not compatible with enabled ASLR" to standard error and exits with status 0 (yes, 0 !!!). If I understand things correctly a similar problem was also on FreeBSD with PIE and the bug wasn't fixed until November of last year: https://github.com/llvm/llvm-project/commit/7440e4ed85aa992718d4b5ccd1c97724bc3bdd2c It might also be worth looking into the other Die() calls in the latest sanitizer_linux.cpp: https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
Re: doc: Document the stack-trace and abort-debug modules
On 2024-07-18 05:25, Bruno Haible wrote: The stack-trace and abort-debug modules are now in a state where they can be documented. Done as follows: Thanks. Are these modules safe to use in signal handlers? I suspect that Emacs would need that, in order to use them. Either way, it should be documented. Similarly, in POSIX an 'abort ()' call is async-signal-safe; is that still true if the abort-debug module is used? This should be documented.
[PATCH] strnlen: port to Android 5.0 (API 21)
This is needed for GNU Emacs, which attempts to port to these old Android versions. * m4/strnlen.m4 (AC_FUNC_STRNLEN): Replace if Autoconf 2.72 or earlier, with code that detects the Android problem with strnlen. This version works around some further bugs in the test, notably, misplaced 'volatile' and need for volatile in the AIX 4.3 bug check too. --- ChangeLog| 11 +++ doc/posix-functions/strnlen.texi | 10 +++ m4/strnlen.m4| 51 +++- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index cb05631a21..08322a3aca 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2024-07-15 Paul Eggert + + strnlen: port to Android 5.0 (API 21) + This is needed for GNU Emacs, which attempts to port to these + old Android versions. + * m4/strnlen.m4 (AC_FUNC_STRNLEN): Replace if Autoconf 2.72 or + earlier, with code that detects the Android problem with strnlen. + This version works around some further bugs in the test, notably, + misplaced 'volatile' and need for volatile in the AIX 4.3 bug + check too. + 2024-07-15 Bruno Haible manywarnings: Don't enable -Wsystem-headers. diff --git a/doc/posix-functions/strnlen.texi b/doc/posix-functions/strnlen.texi index e305a41c8c..554ac12213 100644 --- a/doc/posix-functions/strnlen.texi +++ b/doc/posix-functions/strnlen.texi @@ -9,15 +9,15 @@ Gnulib module: strnlen Portability problems fixed by Gnulib: @itemize @item +On some platforms, calls like @code{strnlen (s, maxlen)} can crash if +@var{s} is null-terminated but address arithmetic overflows +(i.e., @code{s + maxlen < s}): +Android 5.0. +@item This function is missing on some platforms: Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, HP-UX 11, Solaris 10, mingw. @end itemize Portability problems not fixed by Gnulib: @itemize -@item -On some platforms, calls like @code{strnlen (s, maxlen)} can crash if -@var{s} is null-terminated but address arithmetic overflows -(i.e., @code{s + maxlen < s}): -Android 5.0. @end itemize diff --git a/m4/strnlen.m4 b/m4/strnlen.m4 index b4d2778524..83a75c0c32 100644 --- a/m4/strnlen.m4 +++ b/m4/strnlen.m4 @@ -1,11 +1,60 @@ # strnlen.m4 -# serial 14 +# serial 15 dnl Copyright (C) 2002-2003, 2005-2007, 2009-2024 Free Software Foundation, dnl Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. +m4_version_prereq([2.73], [], [ +# Replace AC_FUNC_STRNLEN from Autoconf 2.72 and earlier, +# which does not check for Android strnlen bugs. + +AC_DEFUN([AC_FUNC_STRNLEN], +[AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])dnl +AC_CACHE_CHECK([for working strnlen], [ac_cv_func_strnlen_working], +[AC_RUN_IFELSE( + [AC_LANG_PROGRAM( + [AC_INCLUDES_DEFAULT + [/* Use pstrnlen to test; 'volatile' prevents the compiler + from optimizing the strnlen calls away. */ +size_t (*volatile pstrnlen) (char const *, size_t) = strnlen; +char const s[] = "foobar"; +int s_len = sizeof s - 1; + ]], + [[ +/* AIX 4.3 is buggy: strnlen (S, 1) == 3. */ +int i; +for (i = 0; i < s_len + 1; ++i) + { +int expected = i <= s_len ? i : s_len; +if (pstrnlen (s, i) != expected) + return 1; + } + +/* Android 5.0 (API 21) strnlen ("", SIZE_MAX) incorrectly crashes. */ +if (pstrnlen ("", -1) != 0) + return 1;]])], + [ac_cv_func_strnlen_working=yes], + [ac_cv_func_strnlen_working=no], + [AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT], + [[#if defined _AIX && !defined _AIX51 +#error "AIX pre 5.1 is buggy" + #endif + #ifdef __ANDROID__ +#include +#if __ANDROID_API__ < 22 + #error "Android API < 22 is buggy" +#endif + #endif + ]])], + [ac_cv_func_strnlen_working=yes], + [ac_cv_func_strnlen_working=no])])]) +test $ac_cv_func_strnlen_working = no && AC_LIBOBJ([strnlen]) +])# AC_FUNC_STRNLEN +]) + AC_DEFUN([gl_FUNC_STRNLEN], [ AC_REQUIRE([gl_STRING_H_DEFAULTS]) -- 2.43.0
Re: CI with clang 18 ?
On 7/13/24 09:15, Bruno Haible wrote: In the last two years, I got the feeling that clang is adopting new ISO C features more quickly than GCC (seen with the K&R prototypes removal in clang 16, now with ckd_add in clang 18). I don't have that feeling. I would put it differently, since GCC has supported the ckd_add stuff for some time, and has merely been reluctant to refuse to support K&R style function definitions (which C23 still allows as an extension). I would instead say that GCC is more willing to support C23 extensions than Clang is.
Re: timespec_sub bug fix
On 7/13/24 04:20, Collin Funk wrote: Both type2 and type3 shall be any integer type other than "plain" char, bool, a bit-precise integer type, or an enumerated type, and they need not be the same. Yes, and it's annoying that you can't use 'bool' there. In theory, for example, it means you can't do time_t arithmetic with ckd_add because time_t might be one of the prohibited types. If we had to worry about that possibility our code would be complicated needlessly - so I don't worry about it. I don't know why that limitation is there. Although I was tempted to fix this particular issue with the one-byte fix of inserting "+", e.g., ckd_sub (&rs, rs, borrow) -> ckd_sub (&rs, rs, +borrow), I stuck with the bool -> int fix of the original reporter.
Re: Integer overflows in memchr
On 6/30/24 13:14, Po Lu wrote: I think there should be a trivial test for a functional strnlen in strnlen.m4, since it would be terrible to duplicate what ought to be the responsibility of Gnulib in Emacs's configure.ac. Here's a first cut at doing that, as a patch to Emacs master that I have not installed. Could you please give it a try? If it works I can migrate it into Gnulib and Autoconf. The new code in strnlen.m4 isn't quite the same as the Emacs configure.ac code it replaces, but I hope it's good enough. I should be more portable as it doesn't depend on Emacs configure.ac vars. From 2e8c14206b1a8e3f8bcf34639b3d65861741bd64 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 12 Jul 2024 17:23:30 +0100 Subject: [PATCH] Improve strnlen bug detection The workaround for the Android 5.0 (API 21) strnlen bug is now done in a different way in m4/strnlen.m4, with the idea of migrating that into Gnulib and then to Autoconf. * configure.ac (ORIGINAL_AC_FUNC_STRNLEN, AC_FUNC_STRNLEN): Remove. * m4/strnlen.m4 (AC_FUNC_STRNLEN): Replace if Autoconf 2.72 or earlier, with code that detects the Android problem with strnlen. This version works around some further bugs in the test, notably, misplaced 'volatile' and need for volatile in the AIX 4.3 bug check too. --- configure.ac | 24 m4/strnlen.m4 | 51 ++- 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/configure.ac b/configure.ac index 54c46151bd5..b53224e764d 100644 --- a/configure.ac +++ b/configure.ac @@ -1611,30 +1611,6 @@ AC_DEFUN [HAVE_OFF64_T=1 AC_SUBST([HAVE_OFF64_T])]) -# `strnlen' cannot accept nlen greater than the size of the object S -# on Android 5.0 and earlier. -m4_define([ORIGINAL_AC_FUNC_STRNLEN], m4_defn([AC_FUNC_STRNLEN])) -AC_DEFUN([AC_FUNC_STRNLEN], [ -AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])dnl -AC_REQUIRE([AC_CANONICAL_HOST])dnl -AC_CACHE_CHECK([for strnlen capable of accepting large limits], - [emacs_cv_func_strnlen_working], - [AC_RUN_IFELSE([AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT], [[ - volatile size_t (*strnlen_pointer) (const char *s, size_t) = &strnlen; - if ((*strnlen_pointer) ("", -1) != 0) -return 1; - return 0; -]])],[emacs_cv_func_strnlen_working=yes], - [emacs_cv_func_strnlen_working=no], - [# Guess no on Android 21 and earlier, yes elsewhere. - AS_IF([test -n "$ANDROID_SDK" && test "$ANDROID_SDK" -lt 22], -[emacs_cv_func_strnlen_working=no], - [emacs_cv_func_strnlen_working='guessing yes'])])]) -AS_IF([test "$emacs_cv_func_strnlen_working" != "no"], - [ORIGINAL_AC_FUNC_STRNLEN], - [ac_cv_func_strnlen_working=no - AC_LIBOBJ([strnlen])])]) - # Initialize gnulib right after choosing the compiler. dnl Amongst other things, this sets AR and ARFLAGS. gl_EARLY diff --git a/m4/strnlen.m4 b/m4/strnlen.m4 index b4d2778524e..83a75c0c327 100644 --- a/m4/strnlen.m4 +++ b/m4/strnlen.m4 @@ -1,11 +1,60 @@ # strnlen.m4 -# serial 14 +# serial 15 dnl Copyright (C) 2002-2003, 2005-2007, 2009-2024 Free Software Foundation, dnl Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. +m4_version_prereq([2.73], [], [ +# Replace AC_FUNC_STRNLEN from Autoconf 2.72 and earlier, +# which does not check for Android strnlen bugs. + +AC_DEFUN([AC_FUNC_STRNLEN], +[AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])dnl +AC_CACHE_CHECK([for working strnlen], [ac_cv_func_strnlen_working], +[AC_RUN_IFELSE( + [AC_LANG_PROGRAM( + [AC_INCLUDES_DEFAULT + [/* Use pstrnlen to test; 'volatile' prevents the compiler + from optimizing the strnlen calls away. */ +size_t (*volatile pstrnlen) (char const *, size_t) = strnlen; +char const s[] = "foobar"; +int s_len = sizeof s - 1; + ]], + [[ +/* AIX 4.3 is buggy: strnlen (S, 1) == 3. */ +int i; +for (i = 0; i < s_len + 1; ++i) + { +int expected = i <= s_len ? i : s_len; +if (pstrnlen (s, i) != expected) + return 1; + } + +/* Android 5.0 (API 21) strnlen ("", SIZE_MAX) incorrectly crashes. */ +if (pstrnlen ("", -1) != 0) + return 1;]])], + [ac_cv_func_strnlen_working=yes], + [ac_cv_func_strnlen_working=no], + [AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT], + [[#if defined _AIX && !defined _AIX51 +#error "AIX pre 5.1 is buggy" + #endif + #ifdef __ANDROID__ +#include +#if __ANDROID_API__ < 22 + #error "Android API < 22 is buggy" +#endif + #endif + ]])], + [a
Re: Integer overflows in memchr
On 7/11/24 17:50, Eric Blake wrote: In contrast, it's not OK to call memchr ("", 0, SIZE_MAX). ...this claim appears to be unsupported. Both C23 and POSIX 2024 state that memchr() "...shall behave as if it reads the characters sequentially and stops as soon as a matching character is found", Thanks for correcting me. Evidently I was relying on old memory, as that phrase was added in C11. In penance, I'll mention that there should be similar wording for strncmp. E.g., strncmp ("a", "b", SIZE_MAX) should be valid, although the C and POSIX standards don't make this crystal clear.
Re: Integer overflows in memchr
On 7/9/24 22:03, Eric Blake wrote: https://www.austingroupbugs.net/view.php?id=1834#c6830 The current draft of proposed wording would have the C standard state: 2 The strnlen function counts not more than n characters (a null character and characters that follow it are not counted) in the array to which s points. At most the first n characters of s shall be accessed by strnlen. at which point, strnlen("", SIZE_MAX)_is_ allowed to_access_ beyond the NUL byte, No it wouldn't, because strnlen must stop counting at the first null byte. If this point isn't made clear in the current proposal, it should be made clear. Lots of user code relies on strnlen doing the right thing even if the string is shorter than n. In practice implementations that screw up in this area, and are incompatible with glibc etc., are deemed broken and are fixed. The standard should not allow further breakage.
Re: We can not run gnulib-tool in the MinGW.
On 7/8/24 06:22, anlex N wrote: 1. After I make successfully, I want to place those built files to /e/workspace/github.com/gnu/gnulib/my-gnulib/me/build/make/built. I don't know why that would be useful, but you could use 'cp' to copy them whereever you want. If the goal is to create some sort of library that you can reuse, gnulib-tool isn't set up for that directly. It is a source-code library only. If you want to create an object-code library you must do it yourself, following the instructions in the manual (or looking at other examples of how it's done). 2. I have read all parts of the manual. Because make install does not work, I also tried this command, but sigprocmask still is not recognized. [image: image.png] Please use text to report commands that run and fail, instead of images. This is partly for convenience of the reader, partly for security reasons. Thanks.
ssh into Solaris 10 [was: We can not run gnulib-tool in the MinGW.]
On 7/6/24 23:52, Collin Funk wrote: I can't even ssh into the compile farm Solaris 10 machine without compiling a custom ssh client: $ ssh cfarm210.cfarm.net Bad server host key: Invalid key length Don't know what OpenSSH version you normally run, but with OpenSSH_9.6p1 this configuration works for me: Host cfarm210 HostName %h.cfarm.net HostKeyAlgorithms +ssh-rsa PubkeyAcceptedAlgorithms +ssh-rsa RequiredRSASize 1024 KexAlgorithms +diffie-hellman-group1-sha1 With OpenSSH_8.9p1 I omit the RequiredRSASize line. Isn't SSH configuration wonderful?
Re: We can not run gnulib-tool in the MinGW.
On 7/6/24 17:07, Bruno Haible wrote: are we OK to drop the ability to run gnulib-tool on Solaris 10? I'm OK with it. When I deal with Solaris 10 (hey, our department's admin server is still Solaris 10 sparc!) I run gnulib-tool elsewhere and copy the results to Solaris 10. Does anybody do otherwise? Solaris 10 doesn't have Python 3; Python 2.6.4 (2009) is the default though Python 2.7.18 (2020) and Python 2.3.3 (2003) are also available. Since gnulib-tool's Python implementation requires Python 3.7 or later, Solaris 10 is already kinda on its last legs as far as gnulib-tool is concerned. If it's really necessary to support Solaris 10 sh we can still do it, at the cost of renaming .gnulib-tool.py to gnulib-tool.pydot or something like that. Not sure it's worth the hassle.
Re: We can not run gnulib-tool in the MinGW.
On 7/4/24 13:38, Paul Eggert wrote: exec python3 /home/eggert/src/gnu/gnulib-savannah/./.gnulib-tool.py --list Come to think of it, this can be simplifed to: exec python3 ./.gnulib-tool.py --list which avoids the overhead of computing and using the absolute file name. This will be a minor win even after we get rid of the shell implementation of gnulib-tool. I installed the attached to do that.From 15ec89beae6ded70d4079c5c49861d0b701a353b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 6 Jul 2024 16:41:44 +0200 Subject: [PATCH] gnulib-tool: simplify/speed startup * gnulib-tool, gnulib-tool.py (prog): New var. Use it to simplify and speed up startup in common cases. --- ChangeLog | 6 ++ gnulib-tool| 38 +++--- gnulib-tool.py | 16 ++-- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index b68ed39449..340efb2de3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2024-07-06 Paul Eggert + + gnulib-tool: simplify/speed startup + * gnulib-tool, gnulib-tool.py (prog): New var. Use it to simplify + and speed up startup in common cases. + 2024-07-04 Bruno Haible gitlog-to-changelog: Tweak documentation. diff --git a/gnulib-tool b/gnulib-tool index 789fe916a8..5cfc5bbb52 100755 --- a/gnulib-tool +++ b/gnulib-tool @@ -137,23 +137,35 @@ func_gnulib_dir () gnulib_dir=`echo "$self_abspathname" | sed -e 's,/[^/]*$,,'` } -func_gnulib_dir +# If $progname contains '/' and is not a symlink, it suffices for $prog to be +# the same as $progname with except with basename 'gnulib-tool’; this +# speeds startup and might avoid problems in oddball development environments. +# Otherwise, $prog is the absolute name of the gnulib-tool executable. +if case $progname in + */*) test -h "$0" ;; + esac +then + func_gnulib_dir + prog=$gnulib_dir/gnulib-tool +else + prog=${progname%/*}/gnulib-tool +fi case "$GNULIB_TOOL_IMPL" in '') # Use the Python implementation if a suitable Python version is found # in $PATH. This is the same Python version test as in gnulib-tool.py. if (python3 -c 'import sys; sys.exit(not sys.version_info >= (3,7))') 2>/dev/null; then - exec "$gnulib_dir/gnulib-tool.py" "$@" + exec "$prog.py" "$@" else echo "gnulib-tool: warning: python3 not found or too old, using the slow shell-based implementation" 1>&2 - exec "$gnulib_dir/gnulib-tool.sh" "$@" + exec "$prog.sh" "$@" fi ;; sh) -exec "$gnulib_dir/gnulib-tool.sh" "$@" ;; +exec "$prog.sh" "$@" ;; py) -exec "$gnulib_dir/gnulib-tool.py" "$@" ;; +exec "$prog.py" "$@" ;; sh+py) case " $* " in *" --import"* | *" --add-import"* | *" --remove-import"* | *" --update"* | *" --copy-file"*) @@ -183,10 +195,14 @@ case "$GNULIB_TOOL_IMPL" in func_exit 1 } # Execute gnulib-tool.py in the clone directory. -(cd "$tmp" && "$gnulib_dir/gnulib-tool.py" "$@" >"$tmp-py-out" 2>"$tmp-py-err") +case $prog in + /*) absprog=$prog ;; + *) absprog=$PWD/prog ;; +esac +(cd "$tmp" && "$absprog.py" "$@" >"$tmp-py-out" 2>"$tmp-py-err") pyrc=$? # Execute gnulib-tool.sh in the current directory. -"$gnulib_dir/gnulib-tool.sh" "$@" >"$tmp-sh-out" 2>"$tmp-sh-err" +"$prog.sh" "$@" >"$tmp-sh-out" 2>"$tmp-sh-err" shrc=$? if test $shrc != 0; then if test $pyrc = 0; then @@ -233,10 +249,10 @@ case "$GNULIB_TOOL_IMPL" in # Find another directory name. tmp="$dir-glpy$$" # Execute gnulib-tool.py, creating a different directory. -"$gnulib_dir/gnulib-tool.py" "$@" --dir="$tmp" >"$tmp-py-out" 2>"$tmp-py-err" +"$prog.py" "$@" --dir="$tmp" >"$tmp-py-out" 2>"$tmp-py-err" pyrc=$? # Execute gnulib-tool.sh, creating the intended directory. -"$gnulib_dir/gnulib-tool.sh" "$@" >"$tmp-sh-out" 2>"$tmp-sh-err" +"$prog.sh" "$@" >"$tmp-sh-out" 2>"$tmp-sh-err" shrc=$? if test $shrc != 0; then if test $pyrc = 0; then @@ -274,10 +290,10 @@ case "$GNULIB_TOOL_IMPL" in #
Re: gnulib-tool.py: Fix crash when no ACLOCAL_AMFLAGS is found.
On 6/2/24 14:51, Collin Funk wrote: I don't know enough about Autotools to tell if anything silly can be done using this trick. Not that you would run auto* in a project that you don't trust anyways... Right, but the glitch is worth documenting. I installed the attached into Autoconf.From 3a7aa01e2ada483c5790dc9ea1d761fd75248ff2 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 5 Jul 2024 20:32:40 +0100 Subject: [PATCH] doc: mention ACLOCAL_AMFLAGS and # * doc/autoconf.texi: Mention that ACLOCAL_AMFLAGS cannot use makefile comments. Problem reported by Collin Funk in: https://lists.gnu.org/r/bug-gnulib/2024-06/msg5.html --- doc/autoconf.texi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/autoconf.texi b/doc/autoconf.texi index cb148398..ee3f2f7a 100644 --- a/doc/autoconf.texi +++ b/doc/autoconf.texi @@ -1821,7 +1821,8 @@ If you want @command{autoreconf} to pass flags that are not listed here on to @command{aclocal}, set @code{ACLOCAL_AMFLAGS} in your @file{Makefile.am}. Due to a limitation in the Autoconf implementation these flags currently must be set on a single line in @file{Makefile.am}, without any -backslash-newlines. Also, be aware that future Automake releases might +backslash-newlines or makefile comments. +Also, be aware that future Automake releases might start flagging @code{ACLOCAL_AMFLAGS} as obsolescent, or even remove support for it. @@ -2171,7 +2172,7 @@ Note that if you use @command{aclocal} from an Automake release prior to in your top-level @file{Makefile.am}. Due to a limitation in the Autoconf implementation of @command{autoreconf}, these include directives currently must be set on a single line in @file{Makefile.am}, -without any backslash-newlines. +without any backslash-newlines or makefile comments. @end defmac @prindex @command{config.guess} -- 2.45.2
Re: symlinks to programs
On 7/4/24 20:17, Bruno Haible wrote: PPS. Why do we have both gnulib-tool.py and .gnulib-tool.py? Is this commented in the source code? Yes, there is a large comment about it in .gnulib-tool.py. OK, but that comment explains why .gnulib-tool.py is separate from the main body of the Python program. But why are there two shell scripts gnulib-tool and gnulib-tool.py? Can't the latter be folded into the former? That would be simpler and faster.
Re: symlinks to programs
On 7/4/24 21:49, Simon Josefsson wrote: The Debian gnulib package has a symlink in /usr/bin that goes to /usr/share/gnulib, and there are several source packages that invoke gnulib this way. While this suggest the usage exists, I don't think it necessarily means we must continue support it -- this symlink business has always felt fragile and like a weird way of working to me. Yes it's fragile and weird, but people use it so that's a good argument for keeping it.
Re: symlinks to programs
On 7/4/24 20:17, Bruno Haible wrote: Paul Eggert wrote: PS. There's a lot of machinery in those shell scripts for the minor benefit of letting gnulib-tool be a symlink in your $PATH to the real gnulib-tool. How about if we drop support for this? This is not a "minor" benefit, but a major one. 1) It's documented: <https://www.gnu.org/software/gnulib/manual/html_node/Invoking-gnulib_002dtool.html> Sure, but the proposed patch changes the documentation to say how to get the benefit in a better way. 2) If it was not present, the user would have to write a shell-script wrapper; No, there's no need to write a wrapper; see the proposed patch to the documentation. The new way is better than the old one, because it's a simpler shell command, it requires no change to the file system, and it'll work even on systems that don't support symlinks. 3) The GNU Coding Standards The part of the standard you quoted is intended to apply to GNU programs installed and used in the usual way. gnulib-tool is not such a program. It's not intended to be used anywhere other than the Gnulib source directory, and this reflects Gnulib's unusual role as a source-only library. If people are actually using gnulib-tool via this symlink trick then we should keep supporting it. But I'm truly curious as to who they are and why they want to do that rather than the obvious thing.
Re: We can not run gnulib-tool in the MinGW.
On 7/4/24 14:08, anlex N wrote: I follow Paul Eggert's steps: me@DESKTOP UCRT64 /e/workspace/github.com/gnu/gnulib $ sh -x ./gnulib-tool --create-testdir --dir=/tmp/testdirr --verbose terminfo Oh, I meant for you to run sh -x ./gnulib-tool --list as I thought you said that even a simple --list didn't work. However, it seems that in this run you got further than before so perhaps you fixed something. /e/workspace/github.com/gnu/gnulib/gnulib-tool.py: *** could not patch test-driver script /e/workspace/github.com/gnu/gnulib/gnulib-tool.py: *** Stop. Unfortunately the Python version of gnulib-tool doesn't have a readymade debugging option like 'sh -x' to see what went wrong. But do you have the 'patch' program installed? If not, that might explain the problem. Please check all the programs listed in DEPENDENCIES and make sure they're all installed. As an aside to Collin, I see that patching is done via subprocess.call. Since we're assuming Python 3.7 or later, shouldn't gnulib-tool.py be using subprocess.run instead? That's what the Python documentation recommends, and that way we could capture stderr and report it to the user, which would help debug problems like this.
Re: We can not run gnulib-tool in the MinGW.
On 7/4/24 09:17, Collin Funk wrote: As long as a Python version ≥ 3.7 is installed everything should be fine on that end: Yes, though sometimes Python is misinstalled. When I run "sh -x ./gnulib-tool --list", the last thing it does is: exec /home/eggert/src/gnu/gnulib/./gnulib-tool.py --list and when I run "sh -x /home/eggert/src/gnu/gnulib/./gnulib-tool.py --list", the last thing it does is: exec python3 /home/eggert/src/gnu/gnulib-savannah/./.gnulib-tool.py --list so there should be not much going on other than running Python. Perhaps the bug reporter could try running these "sh -x" commands and letting is know what happens. PS. There's a lot of machinery in those shell scripts for the minor benefit of letting gnulib-tool be a symlink in your $PATH to the real gnulib-tool. How about if we drop support for this? That'd simplify startup quite a bit (if we're lucky it'll even fix the reporter's bug or at least make it easier to diagnose), and there is a better way to get the benefits of that minor feature that doesn't involve so much problematic shell rigamarole. Something like the attached patch, perhaps? I haven't installed it. PPS. Why do we have both gnulib-tool.py and .gnulib-tool.py? Is this commented in the source code?From 6e51dd21e8b62eb5dba5d2164d418fe22ee572a9 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 4 Jul 2024 13:19:23 +0200 Subject: [PATCH] gnulib-tool: simplify startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * gnulib-tool, gnulib-tool.py (func_readlink, func_gnulib_dir): Remove. In main program, use shell substitution rather than external programs, to reduce dependencies on external programs, to simplify, and to speed things up a little. * doc/gnulib-tool.texi (Invoking gnulib-tool): This means that we no longer support the trick of putting a symlink to gnulib-tool somewhere in your PATH, but it’s just as easy to put gnulib-tool in your PATH so document that. --- ChangeLog| 12 + doc/gnulib-tool.texi | 6 +-- gnulib-tool | 113 ++- gnulib-tool.py | 97 +++-- 4 files changed, 35 insertions(+), 193 deletions(-) diff --git a/ChangeLog b/ChangeLog index b19c2a482a..d9ddf46ce5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2024-07-04 Paul Eggert + + gnulib-tool: simplify startup + * gnulib-tool, gnulib-tool.py (func_readlink, func_gnulib_dir): Remove. + In main program, use shell substitution rather than external + programs, to reduce dependencies on external programs, to + simplify, and to speed things up a little. + * doc/gnulib-tool.texi (Invoking gnulib-tool): This means that we + no longer support the trick of putting a symlink to gnulib-tool + somewhere in your PATH, but it’s just as easy to put gnulib-tool + in your PATH so document that. + 2024-07-03 Collin Funk gitlog-to-changelog: Add a new --commit-timezone option. diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi index 0f059e1287..a3b74830ef 100644 --- a/doc/gnulib-tool.texi +++ b/doc/gnulib-tool.texi @@ -24,11 +24,9 @@ simplifies the management of source files, @file{Makefile.am}s and contained in the @code{PATH} variable. It needs to be run directly in the directory that contains the Gnulib source code. You can do this either by specifying the absolute filename of @file{gnulib-tool}, or -you can also use a symbolic link from a place inside your @code{PATH} -to the @file{gnulib-tool} file of your preferred and most up-to-date -Gnulib checkout, like this: +by putting the Gnulib source code directory in your @env{PATH}, like this: @smallexample -$ ln -s $HOME/gnu/src/gnulib.git/gnulib-tool $HOME/bin/gnulib-tool +$ export PATH=$HOME/gnu/src/gnulib.git:$PATH @end smallexample Run @samp{gnulib-tool --help} for information. To get familiar with diff --git a/gnulib-tool b/gnulib-tool index 789fe916a8..0737945b12 100755 --- a/gnulib-tool +++ b/gnulib-tool @@ -50,110 +50,21 @@ func_fatal_error () func_exit 1 } -# func_readlink SYMLINK -# outputs the target of the given symlink. -if (type readlink) > /dev/null 2>&1; then - func_readlink () - { -# Use the readlink program from GNU coreutils. -readlink "$1" - } -else - func_readlink () - { -# Use two sed invocations. A single sed -n -e 's,^.* -> \(.*\)$,\1,p' -# would do the wrong thing if the link target contains " -> ". -LC_ALL=C ls -l "$1" | sed -e 's, -> ,#%%#,' | sed -n -e 's,^.*#%%#\(.*\)$,\1,p' - } -fi - -# func_gnulib_dir -# locates the directory where the gnulib repository lives -# Input: -# - progname name of this program -# Sets variables -# - self_abspathname absolute pathname of gnulib-tool -# - gnulib_dir absolute pathname of gnulib rep
Re: We can not run gnulib-tool in the MinGW.
On 7/4/24 04:26, anlex N wrote: me@DESKTOP UCRT64 /e/workspace/github.com/gnu/gnulib $ ./gnulib-tool --list I run this command in the gnulib source code, but have no output, why? My guess is that you don't have Python installed, or it's installed but is missing some crucial component. If you have some expertise in MinGW - or learn it better, which would be a good idea if you really want to use gnulib-tool under MinGW - it would help if you could see exactly what went wrong. Perhaps gnulib-tool should have better behavior in this situation, though as I don't use MinGW I can't advise how to modify gnulib-tool to do that.
Re: gitlog-to-changelog: Improve handling of time zones.
On 7/3/24 14:48, Bruno Haible wrote: (Btw, is UTC=0 correct? I thought it should be UTC0 It's not portable, although it happens to work on most systems since they default to UTC if TZ has a bogus value. I installed the attached to fix that, and to improve on other issues I noticed in that doc file. The basic idea is to shorten the documentation by giving the reproducible recipe first, and then telling people what line to omit if they don't want reproducibility.From dc2886c010dc34665370be055b2d93ff9aed68d2 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 4 Jul 2024 00:31:33 +0100 Subject: [PATCH] gitlog-to-changelog: improve doc --- doc/gitlog-to-changelog.texi | 70 ++-- 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/doc/gitlog-to-changelog.texi b/doc/gitlog-to-changelog.texi index 137b15fcda..5dc1043e45 100644 --- a/doc/gitlog-to-changelog.texi +++ b/doc/gitlog-to-changelog.texi @@ -12,8 +12,8 @@ @cindex gitlog @cindex changelog -Gnulib have a module @code{gitlog-to-changelog} to parse @code{git log} -output and generate @code{ChangeLog} files, see +Gnulib has a module @code{gitlog-to-changelog} to parse @code{git log} +output and generate @file{ChangeLog} files; see @ifinfo @ref{Change Logs,,,standards}. @end ifinfo @@ -21,61 +21,45 @@ output and generate @code{ChangeLog} files, see @url{https://www.gnu.org/prep/standards/html_node/Change-Logs.html}. @end ifnotinfo -You would typically use it by extending the @code{dist-hook} in the -top-level @code{Makefile.am} like this: +You can use it by extending the @code{dist-hook} rule in the +top-level @file{Makefile.am} like this: @example dist-hook: gen-ChangeLog -... .PHONY: gen-ChangeLog gen-ChangeLog: -$(AM_V_GEN)if test -e .git; then \ -$(top_srcdir)/build-aux/gitlog-to-changelog > \ -$(distdir)/cl-t && \ -@{ rm -f $(distdir)/ChangeLog &&\ - mv $(distdir)/cl-t $(distdir)/ChangeLog; @} \ +$(AM_V_GEN)if test -e .git; then \ + LC_ALL=en_US.UTF-8 TZ=UTC0 \ +$(top_srcdir)/build-aux/gitlog-to-changelog\ + > $(distdir)/ChangeLog.tmp &&\ + mv -f $(distdir)/ChangeLog.tmp $(distdir)/ChangeLog; \ fi @end example See @code{gitlog-to-changelog --help} for complete documentation. -The tool prints timestamps using @code{localtime}, so its output may be -different depending on what locale the developer that runs the tool is -using. If your project desire reproducible ChangeLog files that doesn't -depend on locale settings, use something like the following. +The @code{LC_ALL=en_US.UTF-8 TZ=UTC0} line in this recipe assumes that +you want to generate reproducible @file{ChangeLog} files that do not +depend on the developer's locale and time zone. Omit this line if you +prefer @file{ChangeLog} files that depend on these developer settings. -@example -gen-ChangeLog: -$(AM_V_GEN)if test -e .git; then \ -env LC_ALL=en_US.UTF-8 TZ=UTC=0\ -$(top_srcdir)/build-aux/gitlog-to-changelog > \ -$(distdir)/cl-t && \ -@{ rm -f $(distdir)/ChangeLog &&\ - mv $(distdir)/cl-t $(distdir)/ChangeLog; @} \ -fi -@end example - - -If you wish to limit the ChangeLog entries (perhaps for size issues) to -only contain entries since a particular git tag, use something like the -following: +If you wish to limit the @file{ChangeLog} entries (perhaps for size +issues) to contain only entries since a particular git tag, you can +use a @code{gen-ChangeLog} rule like the following: @example -dist-hook: gen-ChangeLog -... gen_start_ver = 8.31 -.PHONY: gen-ChangeLog gen-ChangeLog: -$(AM_V_GEN)if test -e .git; then \ - log_fix="$(srcdir)/build-aux/git-log-fix"; \ - test -e "$$log_fix" \ -&& amend_git_log="--amend=$$log_fix" \ -|| amend_git_log=; \ - $(top_srcdir)/build-aux/gitlog-to-changelog $$amend_git_log \ --- v$(gen_start_ver)~.. > $(distdir)/cl-t && \ -@{ printf '\n\nSee the source repo for older entries\n' \ - >> $(distdir)/cl-t &&\ - rm -f $(distdir)/ChangeLog &&\ -
Re: Integer overflows in memchr
On 7/1/24 02:16, Po Lu wrote: but Gnulib (and by extension Emacs) does not invoke the Autoconf test: AC_DEFUN([gl_FUNC_STRNLEN], [ AC_REQUIRE([gl_STRING_H_DEFAULTS]) dnl Persuade glibc to declare strnlen(). AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS]) AC_CHECK_DECLS_ONCE([strnlen]) < if test $ac_cv_have_decl_strnlen = no; then HAVE_DECL_STRNLEN=0 else m4_pushdef([AC_LIBOBJ], [:]) dnl Note: AC_FUNC_STRNLEN does AC_LIBOBJ([strnlen]). The line immediately after the last line you quoted is: AC_FUNC_STRNLEN and that invokes the Autoconf test.
Re: Integer overflows in memchr
On 6/30/24 13:18, Po Lu wrote: How many lines remain of my 15-line quota of contributions to Gnulib? I wouldn't worry about the quota; we can solve that problem later as need be. #if defined __ANDROID_API__ && (__ANDROID_API__ < 21) you lose #endif /* defined __ANDROID_API__ && (__ANDROID_API__ < 21) */ Autoconf's existing AC_FUNC_STRNLEN uses AC_RUN_IFELSE so this wouldn't work as-is. Perhaps you could write up a patch against Autoconf? That wouldn't count against the Gnulib quota and, when not cross-compiling, verify (strnlen ("", SIZE_MAX) == 0); This won't work in general, as GCC will optimize away the call to strnlen and presumably other compilers will do the same.
Re: Integer overflows in memchr
On 6/30/24 12:14, Po Lu wrote: I think there should be a trivial test for a functional strnlen in strnlen.m4 Please feel free to write that. It's not something that would be easy for me to do, as I don't know Android. Also, I don't know that it'd be that trivial, as it might involve replacing the AC_FUNC_STRNLEN macro of Autoconf (and we should propagate that fix into Autoconf as well), but I could give advice about the non-Android part of any fix. As I wrote earlier, although I hope we don't need to worry about the small number of users still running Android 5.0, if it's important to support free software on those platforms we can do so.