request for update to z/OS progname

2023-05-30 Thread Mike Fulton
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

2023-05-30 Thread Mike Fulton
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.

2023-05-30 Thread Paul Eggert

Thanks, I installed that.



[PATCH] readline: fix memory leak in replacement readline.

2023-05-30 Thread Nick Bowler
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)

2023-05-30 Thread Paul Eggert

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)

2023-05-30 Thread Bruno Haible
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)

2023-05-30 Thread Paul Eggert

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)

2023-05-30 Thread Paul Eggert

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

2023-05-30 Thread Paul Eggert

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]

2023-05-30 Thread Bruno Haible
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