request for update to z/OS progname
Hello, In getprogname.c, there is a section of code delimited by # elif __MVS__ ... #elif defined __sgi || defined __osf__ I would like to request a patch so that code built in ASCII mode will operate correctly. Currently, the program name returned is always in EBCDIC. This can be fixed with the patch: if (token > 0 && buf.ps_pid == pid) { char *s = strdup (last_component (buf.ps_pathptr)); - if (s) + if (s) { p = s; +#if (__CHARSET_LIB == 1) +__e2a_s(p); +#endif + } break; } } This is lines 213 to 220 of my version of the code - which I believe is current. The __e2a_s function will convert from EBCDIC 1047 to ASCII ISO8859-1. The program name is not locale sensitive. Specifically, I am patching this code for man-db so that it operates correctly, and this would let me eliminate this patch. Thanks, Mike
add __MVS__ to list of systems that should default to UTF-8 in localcharset.c
Hi, I would like to request a patch to localcharset.c to add __MVS__ to the list of systems that should default to UTF-8. On line 1137 to 1139, the code is currently: # if (defined __APPLE__ && defined __MACH__) || defined __BEOS__ || defined __HAIKU__ codeset = "UTF-8"; # else I would like to request the __MVS__ macro be added so that z/OS will default to UTF-8, e.g. # if (defined __APPLE__ && defined __MACH__) || defined __BEOS__ || defined __HAIKU__ || defined __MVS__ codeset = "UTF-8"; # else I am currently using this patch for man-db so that it can correctly render UTF-8 man pages.
Re: [PATCH] readline: fix memory leak in replacement readline.
Thanks, I installed that.
[PATCH] readline: fix memory leak in replacement readline.
The getline function allocates memory that has to be freed by the caller regardless of whether or not the call succeeds. This is the case at least on current GNU libc getline, and also the case with Gnulib's own replacement getline implementation. Gnulib's readline replacement, which calls getline internally, leaks this memory whenever that call returns -1 (either at EOF or some error). * lib/readline.c (readline): free allocated memory after getline failure. --- lib/readline.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/readline.c b/lib/readline.c index 8e24090c46ad..9ea9b45c81c6 100644 --- a/lib/readline.c +++ b/lib/readline.c @@ -30,6 +30,7 @@ #include "readline.h" #include +#include #include char * @@ -45,7 +46,10 @@ readline (const char *prompt) } if (getline (&out, &size, stdin) < 0) -return NULL; +{ + free(out); + return NULL; +} while (*out && (out[strlen (out) - 1] == '\r' || out[strlen (out) - 1] == '\n')) -- 2.39.2
Re: error: Support the compiler's control flow analysis better (was: copy-file: Silence gcc warnings)
On 5/30/23 15:06, Bruno Haible wrote: In terms of functions calls, I don't think it makes a difference, whether exit() gets called from within the error() invocation [1] or after the error() invocation. It makes the calling code a bit smaller, e.g., it avoids GCC having to generate code to save and restore registers in the caller, along with issuing two calls instead of one. Admittedly not a huge deal. I also vaguely think that perhaps GCC will find the resulting code easier to analyze. Although this is just a guess, GCC is kinda buggy in this area and I hope the smaller code avoid bugs. + __gl_error_call (rpl_error, status, __VA_ARGS) What is __VA_ARGS ? Did you mean __VA_ARGS__ ? Yes I did. Thanks for catching that typo. I installed a fix.
Re: error: Support the compiler's control flow analysis better (was: copy-file: Silence gcc warnings)
Paul Eggert wrote: > > +# define error(status, ...) \ > > + ((error)(0, __VA_ARGS__), (status) ? exit (status) : (void)0) > > We can do a bit better than that by using 'unreachable ()' instead of > 'exit (status)', and passing 'status' (instead of 0) to the underlying > error function. This saves a function call In terms of functions calls, I don't think it makes a difference, whether exit() gets called from within the error() invocation [1] or after the error() invocation. [1] lib/error.c:285 > Also, it's better to not evaluate 'status' twice. Not that I think > 'status' should have side effects or even that it does have side effects > in any Gnulib-using code, just that it's more hygienic in case some > caller foolishly puts side effects there. Indeed, having it evaluate only once is better. Thanks. > + __gl_error_call (rpl_error, status, __VA_ARGS) What is __VA_ARGS ? Did you mean __VA_ARGS__ ? Bruno
Re: error: Support the compiler's control flow analysis better (was: copy-file: Silence gcc warnings)
On 5/27/23 13:53, Bruno Haible wrote: +# define error(status, ...) \ + ((error)(0, __VA_ARGS__), (status) ? exit (status) : (void)0) We can do a bit better than that by using 'unreachable ()' instead of 'exit (status)', and passing 'status' (instead of 0) to the underlying error function. This saves a function call and should still pacify GCC. Also, it's better to not evaluate 'status' twice. Not that I think 'status' should have side effects or even that it does have side effects in any Gnulib-using code, just that it's more hygienic in case some caller foolishly puts side effects there. I installed the attached further patches into Gnulib to try to address these issues.From 8b21ff255996a518244a5635e36ea58e21f818be Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 30 May 2023 12:49:20 -0700 Subject: [PATCH 1/2] =?UTF-8?q?error:=20don=E2=80=99t=20evaluate=20status?= =?UTF-8?q?=20arg=20twice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids potential issues if the first argument has a side effect. * lib/error.in.h (__gl_error_call): New macro, which evaluates its status arg only once, by using a statement expression if GNU C - the only platform we need to worry about pacifying - and by simply calling ‘error’ otherwise. (error, error_at_line): Use it. --- ChangeLog | 10 ++ lib/error.in.h | 20 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 91a6135f8a..f70eeef893 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2023-05-30 Paul Eggert + + error: don’t evaluate status arg twice + This avoids potential issues if the first argument has a side effect. + * lib/error.in.h (__gl_error_call): New macro, which evaluates its + status arg only once, by using a statement expression if GNU C - + the only platform we need to worry about pacifying - and by simply + calling ‘error’ otherwise. + (error, error_at_line): Use it. + 2023-05-28 Bruno Haible warnings, manywarnings: Assume autoconf >= 2.64. diff --git a/lib/error.in.h b/lib/error.in.h index 70fb132133..a5f49e4143 100644 --- a/lib/error.in.h +++ b/lib/error.in.h @@ -49,6 +49,18 @@ # define _GL_ATTRIBUTE_SPEC_PRINTF_ERROR _GL_ATTRIBUTE_SPEC_PRINTF_SYSTEM #endif +#ifdef __GNUC__ +# define __gl_error_call(function, status, ...) \ +({ \ + int const __errstatus = status; \ + (function) (0, __VA_ARGS__); \ + __errstatus != 0 ? exit (__errstatus) : (void) 0; \ +}) +#else +# define __gl_error_call(function, status, ...) \ +(function) (status, __VA_ARGS__) +#endif + #ifdef __cplusplus extern "C" { #endif @@ -69,7 +81,7 @@ _GL_CXXALIAS_RPL (error, void, # ifndef _GL_NO_INLINE_ERROR # undef error # define error(status, ...) \ - ((rpl_error)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0) + __gl_error_call (rpl_error, status, __VA_ARGS) # endif #else # if ! @HAVE_ERROR@ @@ -81,7 +93,7 @@ _GL_CXXALIAS_SYS (error, void, (int __status, int __errnum, const char *__format, ...)); # ifndef _GL_NO_INLINE_ERROR # define error(status, ...) \ - ((error)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0) + __gl_error_call (error, status, __VA_ARGS__) # endif #endif #if __GLIBC__ >= 2 @@ -105,7 +117,7 @@ _GL_CXXALIAS_RPL (error_at_line, void, # ifndef _GL_NO_INLINE_ERROR # undef error_at_line # define error_at_line(status, ...) \ - ((rpl_error_at_line)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0) + __gl_error_call (rpl_error_at_line, status, __VA_ARGS__) # endif #else # if ! @HAVE_ERROR_AT_LINE@ @@ -119,7 +131,7 @@ _GL_CXXALIAS_SYS (error_at_line, void, unsigned int __lineno, const char *__format, ...)); # ifndef _GL_NO_INLINE_ERROR # define error_at_line(status, ...) \ - ((error_at_line)(0, __VA_ARGS__), (status) != 0 ? exit (status) : (void)0) + __gl_error_call (error_at_line, status, __VA_ARGS__) # endif #endif _GL_CXXALIASWARN (error_at_line); -- 2.40.1 From 47811d1ac24e38e8842dc37e2e3cd3b4120338ad Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 30 May 2023 14:38:41 -0700 Subject: [PATCH 2/2] =?UTF-8?q?error:=20don=E2=80=99t=20call=20=E2=80=98ex?= =?UTF-8?q?it=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let the underlying functions call ‘exit’, instead of having the Gnulib replacement macros do it. Use ‘unreachable’ to tell the compiler that those functions exit when the status is nonzero. This saves a function call. * lib/error.in.h: Include stddef.h, not stdlib.h. (__gl_error_call): Rely on the function to exit, using ‘unreachable’ to tell the compiler that the function does not return. * modules/error (Depends-on): Add stddef. --- ChangeLog | 10 ++ lib/error.in.h | 10 +- modules/error | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git
Re: error: Support the compiler's control flow analysis better (was: copy-file: Silence gcc warnings)
On 5/28/23 06:07, Pádraig Brady wrote: There still is a gotcha (hit in dd.c in coreutils) where if you define an error macro yourself you get a macro redefinition error, I see you fixed that by adding a quick "#define _GL_NO_INLINE_ERROR" to coreutils/src/dd.c. It's a bit cleaner to fix the underlying naming problem instead, so that dd.c need not define the Gnulib internals macro (or its own quirky error macro), so I installed the attached to coreutils to do that. From 48f5a39872acbf3f23f02cf3216a6e30d00db671 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 30 May 2023 14:24:46 -0700 Subject: [PATCH] =?UTF-8?q?dd:=20fix=20=E2=80=98error=E2=80=99=20name=20is?= =?UTF-8?q?sue=20without=20macros?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/dd.c (_GL_NO_INLINE_ERROR): Remove; no longer needed. (diagnose): Rename from nl_error and omit first arg since it is always zero. All uses changed. (error): Remove macro. --- src/dd.c | 109 ++- 1 file changed, 52 insertions(+), 57 deletions(-) diff --git a/src/dd.c b/src/dd.c index f41966969..8fcfdd3db 100644 --- a/src/dd.c +++ b/src/dd.c @@ -17,7 +17,6 @@ /* Written by Paul Rubin, David MacKenzie, and Stuart Kemp. */ #include -#define _GL_NO_INLINE_ERROR /* Avoid gnulib's error macro. */ #include #include @@ -515,11 +514,12 @@ maybe_close_stdout (void) _exit (EXIT_FAILURE); } -/* Like the 'error' function but handle any pending newline. */ +/* Like the 'error' function but handle any pending newline, + and do not exit. */ -ATTRIBUTE_FORMAT ((__printf__, 3, 4)) +ATTRIBUTE_FORMAT ((__printf__, 2, 3)) static void -nl_error (int status, int errnum, char const *fmt, ...) +diagnose (int errnum, char const *fmt, ...) { if (0 < progress_len) { @@ -529,12 +529,10 @@ nl_error (int status, int errnum, char const *fmt, ...) va_list ap; va_start (ap, fmt); - verror (status, errnum, fmt, ap); + verror (0, errnum, fmt, ap); va_end (ap); } -#define error nl_error - void usage (int status) { @@ -1138,12 +1136,12 @@ iread (int fd, char *buf, idx_t size) { idx_t prev = prev_nread; if (status_level != STATUS_NONE) -error (0, 0, ngettext (("warning: partial read (%td byte); " +diagnose (0, ngettext (("warning: partial read (%td byte); " "suggest iflag=fullblock"), ("warning: partial read (%td bytes); " "suggest iflag=fullblock"), select_plural (prev)), - prev); + prev); warn_partial_read = false; } } @@ -1188,8 +1186,8 @@ iwrite (int fd, char const *buf, idx_t size) int old_flags = fcntl (STDOUT_FILENO, F_GETFL); if (fcntl (STDOUT_FILENO, F_SETFL, old_flags & ~O_DIRECT) != 0 && status_level != STATUS_NONE) -error (0, errno, _("failed to turn off O_DIRECT: %s"), - quotef (output_file)); +diagnose (errno, _("failed to turn off O_DIRECT: %s"), + quotef (output_file)); /* Since we have just turned off O_DIRECT for the final write, we try to preserve some of its semantics. */ @@ -1263,7 +1261,7 @@ write_output (void) w_bytes += nwritten; if (nwritten != output_blocksize) { - error (0, errno, _("writing to %s"), quoteaf (output_file)); + diagnose (errno, _("writing to %s"), quoteaf (output_file)); if (nwritten != 0) w_partial++; quit (EXIT_FAILURE); @@ -1392,8 +1390,9 @@ parse_symbols (char const *str, struct symbol_value const *table, if (! entry->symbol[0]) { idx_t slen = strcomma ? strcomma - str : strlen (str); - error (0, 0, "%s: %s", _(error_msgid), - quotearg_n_style_mem (0, locale_quoting_style, str, slen)); + diagnose (0, "%s: %s", _(error_msgid), +quotearg_n_style_mem (0, locale_quoting_style, + str, slen)); usage (EXIT_FAILURE); } } @@ -1456,10 +1455,9 @@ parse_integer (char const *str, strtol_error *invalid) else { if (result == 0 && STRPREFIX (str, "0x")) -error (0, 0, - _("warning: %s is a zero multiplier; " - "use %s if that is intended"), - quote_n (0, "0x"), quote_n (1, "00x")); +diagnose (0, _("warning: %s is a zero multiplier; " + "use %s if that is intended"), + quote_n (0, "0x"), quote_n (1, "00x")); e = LONGINT_OK; } } @@ -1500,8 +1498,7 @@ scanargs (int argc, char *const *argv) if (val == NULL) { - error (0, 0, _("unr
Re: [PATCH] file-has-acl: avoid warning from bleeding-edge GCC
On 5/28/23 11:49, Jim Meyering wrote: That would also solve the problem, but at a potential code-gen cost, as you imply: code-gen for a few rare new and modified functions may end up being degraded. I would endorse this approach if that GCC bug is still unresolved in say October. Sounds good to me, thanks.
Re: (lib/error.h.in) New warnings when compiling "groff" [-Wredundant-decls]
Bjarni Ingi Gislason wrote: > Debian GNU/Linux 12 (bookworm) > Linux 6.1.27-1 x86_64 GNU/Linux > gcc (Debian 12.2.0-14) 12.2.0 > > [...] > CC lib/libgnu_a-openat-die.o Next time, use "make V=1" before reporting something. The command-line options passed to the compiler _are_ relevant. > In file included from ../lib/openat-die.c:25: > ./lib/error.h:28:3: warning: #include_next is a GCC extension >28 | # include_next > | ^~~~ > In file included from ../lib/openat.h:28, > from ../lib/openat-die.c:20: > ./lib/error.h:418:1: warning: redundant redeclaration of '_gl_cxxalias_dummy' > [-Wredundant-decls] > 418 | _GL_CXXALIAS_SYS (error, void, > | ^~~~ These warnings occur with any gnulib-overridden .h file / function. Therefore my advice is to not specify the corresponding warning options. It certainly doesn't hurt much, since '-Wredundant-decls' is not a warning option that frequently points to real bugs. Bruno