[PATCH v3] Add internal implementations for argp.h, err.h, and error.h functions

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

Hi, Paul,

On Thu, 06 Dec 2018, Gabriel F. T. Gomes wrote:

>Paul,
>
>Would you mind having a look at this patch?  After Joseph mentioned that
>I should work with you to synchronize the implementations in glibc and
>gnulib, I found that I made a mistake in the first version (I blindly
>changed code in some #ifndef _LIBC blocks.  I don't know what else to
>look for.

I took a look at the differences between the gnulib and glibc versions of
argp-help.c and error.c to try and come up with more interesting questions
about the synchronization...  Here they are:

argp.h:

In argp-help.c, there are two new functions that are not exported by
glibc: __argp_error_internal and __argp_failure_internal (which are
called from their exported counterparts: argp_error and argp_failure).
Are these new functions a problem for gnulib?
(Other changes are contained within `#ifdef _LIBC' clauses)
                                                                         
err.h:

Not in gnulib, so I couldn't think of anything to stress here.
                                                                         
error.h:

Unlike the changes to argp_error and argp_failure, in error.c, I did not
add an internal version of error_tail (I simply changed its interface,
because it is not exported by glibc).  Is this a problem for gnulib?


Besides that, I noticed that I had not removed the inclusion of libioP.h
in v2.  This is a residue from v1 and not needed, so I'm attaching v3 with
those hunks removed.

Thanks,
Gabriel


From 62215fd112a2fc5d4e9cfe9b68cfe439cbd34746 Mon Sep 17 00:00:00 2001
From: "Gabriel F. T. Gomes" <gabriel@inconstante.eti.br>
Date: Wed, 6 Jun 2018 11:48:49 -0300
Subject: [PATCH v3] Add internal implementations for argp.h, err.h, and
 error.h functions

Changes since v2:

  - Removed inclusion of libio/libioP.h from err.c and error.c, because
    they no longer make calls to __vfprinf_internal or
    __vfwprintf_internal (this is a residue from v1).

Changes since v1:

  - When _LIBC is not defined, revert hunks that replaced calls to
    vfprintf with calls to __vfprintf_internal.
  - Updated to use __vfxprintf instead of convert_and_print (deleted).
  - Added mode_flags parameter to the new functions (__vfxprintf and
    locked_vfxprintf).

-- 8< --
Add internal implementations for argp.h, err.h, and error.h functions

Since the introduction of explicit flags in the internal implementation
of the printf family of functions, the 'mode' parameter can be used to
select which format long double parameters have (with the mode flag:
PRINTF_LDBL_IS_DBL).  This patch uses this feature in the implementation
of some functions in argp.h, err.h, and error.h (only those that take a
format string and positional parameters).  Future patches will add
support for 'nldbl' and 'ieee128' versions of these functions.

Tested for powerpc64le.

	* argp/argp-help.c (__argp_error_internal): New function,
	renamed from __argp_error, but that takes a 'mode_flags'
	parameter to control the format of long double parameters.
	(__argp_error): Converted into a call __argp_error_internal.
	(__argp_failure_internal): New function, renamed from
	__argp_failure, but that takes a 'mode_flags' parameter.
	(__argp_failure): Converted into a call __argp_failure_internal.
	* misc/err.c: (__vwarnx_internal): New function, renamed from
	vwarnx, but that takes a 'mode_flags' parameter.
	(vwarnx): Converted into a call to __vwarnx_internal.
	(__vwarn_internal): New function, renamed from vwarn, but that
	takes a 'mode_flags' parameter.
	(vwarn): Converted into a call to __vwarn_internal.
	* misc/error.c: (error_tail): Add 'mode_flags' parameter. Update
	call to __vfxprintf with 'mode_flags'.
	(__error_internal): New function, renamed from error, but that
	takes a 'mode_flags' parameter.
	(error): Converted into a call to __error_internal.
	(__error_at_line_internal): New function, renamed from
	error_at_line, but that takes a 'mode_flags' parameter.
	(error_at_line): Converted into a call to
	__error_at_line_internal.
	* include/stdio.h (__vfxprintf): Add mode_flags parameter.
	* stdio-common/fxprintf.c (locked_vfxprintf, __vfxprintf):
	Likewise.
---
 argp/argp-help.c        | 40 ++++++++++++++++++++++++----------------
 include/stdio.h         |  3 ++-
 misc/err.c              | 23 ++++++++++++++++++-----
 misc/error.c            | 45 ++++++++++++++++++++++++++++++---------------
 stdio-common/fxprintf.c | 16 +++++++++-------
 5 files changed, 83 insertions(+), 44 deletions(-)

diff --git a/argp/argp-help.c b/argp/argp-help.c
index 09c76734d3..616e3ed1b3 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -1750,7 +1750,8 @@ weak_alias (__argp_state_help, argp_state_help)
    by the program name and `:', to stderr, and followed by a `Try ... --help'
    message, then exit (1).  */
 void
-__argp_error (const struct argp_state *state, const char *fmt, ...)
+__argp_error_internal (const struct argp_state *state, const char *fmt,
+		       va_list ap, unsigned int mode_flags)
 {
   if (!state || !(state->flags & ARGP_NO_ERRS))
     {
@@ -1758,18 +1759,14 @@ __argp_error (const struct argp_state *state, const char *fmt, ...)
 
       if (stream)
 	{
-	  va_list ap;
-
 #if _LIBC || (HAVE_FLOCKFILE && HAVE_FUNLOCKFILE)
 	  __flockfile (stream);
 #endif
 
-	  va_start (ap, fmt);
-
 #ifdef _LIBC
 	  char *buf;
 
-	  if (__vasprintf_internal (&buf, fmt, ap, 0) < 0)
+	  if (__vasprintf_internal (&buf, fmt, ap, mode_flags) < 0)
 	    buf = NULL;
 
 	  __fxprintf (stream, "%s: %s\n",
@@ -1789,14 +1786,20 @@ __argp_error (const struct argp_state *state, const char *fmt, ...)
 
 	  __argp_state_help (state, stream, ARGP_HELP_STD_ERR);
 
-	  va_end (ap);
-
 #if _LIBC || (HAVE_FLOCKFILE && HAVE_FUNLOCKFILE)
 	  __funlockfile (stream);
 #endif
 	}
     }
 }
+void
+__argp_error (const struct argp_state *state, const char *fmt, ...)
+{
+  va_list ap;
+  va_start (ap, fmt);
+  __argp_error_internal (state, fmt, ap, 0);
+  va_end (ap);
+}
 #ifdef weak_alias
 weak_alias (__argp_error, argp_error)
 #endif
@@ -1810,8 +1813,9 @@ weak_alias (__argp_error, argp_error)
    *parsing errors*, and the former is for other problems that occur during
    parsing but don't reflect a (syntactic) problem with the input.  */
 void
-__argp_failure (const struct argp_state *state, int status, int errnum,
-		const char *fmt, ...)
+__argp_failure_internal (const struct argp_state *state, int status,
+			 int errnum, const char *fmt, va_list ap,
+			 unsigned int mode_flags)
 {
   if (!state || !(state->flags & ARGP_NO_ERRS))
     {
@@ -1833,13 +1837,10 @@ __argp_failure (const struct argp_state *state, int status, int errnum,
 
 	  if (fmt)
 	    {
-	      va_list ap;
-
-	      va_start (ap, fmt);
 #ifdef _LIBC
 	      char *buf;
 
-	      if (__vasprintf_internal (&buf, fmt, ap, 0) < 0)
+	      if (__vasprintf_internal (&buf, fmt, ap, mode_flags) < 0)
 		buf = NULL;
 
 	      __fxprintf (stream, ": %s", buf);
@@ -1851,8 +1852,6 @@ __argp_failure (const struct argp_state *state, int status, int errnum,
 
 	      vfprintf (stream, fmt, ap);
 #endif
-
-	      va_end (ap);
 	    }
 
 	  if (errnum)
@@ -1889,6 +1888,15 @@ __argp_failure (const struct argp_state *state, int status, int errnum,
 	}
     }
 }
+void
+__argp_failure (const struct argp_state *state, int status, int errnum,
+		const char *fmt, ...)
+{
+  va_list ap;
+  va_start (ap, fmt);
+  __argp_failure_internal (state, status, errnum, fmt, ap, 0);
+  va_end (ap);
+}
 #ifdef weak_alias
 weak_alias (__argp_failure, argp_failure)
 #endif
diff --git a/include/stdio.h b/include/stdio.h
index 1b7da0f74d..6b77a0ddc2 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -124,7 +124,8 @@ extern int __fxprintf (FILE *__fp, const char *__fmt, ...)
      __attribute__ ((__format__ (__printf__, 2, 3))) attribute_hidden;
 extern int __fxprintf_nocancel (FILE *__fp, const char *__fmt, ...)
      __attribute__ ((__format__ (__printf__, 2, 3))) attribute_hidden;
-int __vfxprintf (FILE *__fp, const char *__fmt, __gnuc_va_list)
+int __vfxprintf (FILE *__fp, const char *__fmt, __gnuc_va_list,
+		 unsigned int)
   attribute_hidden;
 
 /* Read the next line from FP into BUFFER, of LENGTH bytes.  LINE will
diff --git a/misc/err.c b/misc/err.c
index b6afe65516..3259ce41c6 100644
--- a/misc/err.c
+++ b/misc/err.c
@@ -38,19 +38,20 @@ extern char *__progname;
 }
 
 void
-vwarnx (const char *format, __gnuc_va_list ap)
+__vwarnx_internal (const char *format, __gnuc_va_list ap,
+		   unsigned int mode_flags)
 {
   flockfile (stderr);
   __fxprintf (stderr, "%s: ", __progname);
   if (format != NULL)
-    __vfxprintf (stderr, format, ap);
+    __vfxprintf (stderr, format, ap, mode_flags);
   __fxprintf (stderr, "\n");
   funlockfile (stderr);
 }
-libc_hidden_def (vwarnx)
 
 void
-vwarn (const char *format, __gnuc_va_list ap)
+__vwarn_internal (const char *format, __gnuc_va_list ap,
+		   unsigned int mode_flags)
 {
   int error = errno;
 
@@ -58,7 +59,7 @@ vwarn (const char *format, __gnuc_va_list ap)
   if (format != NULL)
     {
       __fxprintf (stderr, "%s: ", __progname);
-      __vfxprintf (stderr, format, ap);
+      __vfxprintf (stderr, format, ap, mode_flags);
       __set_errno (error);
       __fxprintf (stderr, ": %m\n");
     }
@@ -69,8 +70,20 @@ vwarn (const char *format, __gnuc_va_list ap)
     }
   funlockfile (stderr);
 }
+
+void
+vwarn (const char *format, __gnuc_va_list ap)
+{
+  __vwarn_internal (format, ap, 0);
+}
 libc_hidden_def (vwarn)
 
+void
+vwarnx (const char *format, __gnuc_va_list ap)
+{
+  __vwarnx_internal (format, ap, 0);
+}
+libc_hidden_def (vwarnx)
 
 void
 warn (const char *format, ...)
diff --git a/misc/error.c b/misc/error.c
index cb0de3af28..88a1fa323d 100644
--- a/misc/error.c
+++ b/misc/error.c
@@ -200,10 +200,11 @@ print_errno_message (int errnum)
 }
 
 static void _GL_ATTRIBUTE_FORMAT_PRINTF (3, 0) _GL_ARG_NONNULL ((3))
-error_tail (int status, int errnum, const char *message, va_list args)
+error_tail (int status, int errnum, const char *message, va_list args,
+	    unsigned int mode_flags)
 {
 #if _LIBC
-  int ret = __vfxprintf (stderr, message, args);
+  int ret = __vfxprintf (stderr, message, args, mode_flags);
   if (ret < 0 && errno == ENOMEM && _IO_fwide (stderr, 0) > 0)
     /* Leave a trace in case the heap allocation of the message string
        failed.  */
@@ -232,10 +233,9 @@ error_tail (int status, int errnum, const char *message, va_list args)
    If ERRNUM is nonzero, print its corresponding system error message.
    Exit with status STATUS if it is nonzero.  */
 void
-error (int status, int errnum, const char *message, ...)
+__error_internal (int status, int errnum, const char *message,
+		  va_list args, unsigned int mode_flags)
 {
-  va_list args;
-
 #if defined _LIBC && defined __libc_ptf_call
   /* We do not want this call to be cut short by a thread
      cancellation.  Therefore disable cancellation for now.  */
@@ -259,9 +259,7 @@ error (int status, int errnum, const char *message, ...)
 #endif
     }
 
-  va_start (args, message);
-  error_tail (status, errnum, message, args);
-  va_end (args);
+  error_tail (status, errnum, message, args, mode_flags);
 
 #ifdef _LIBC
   _IO_funlockfile (stderr);
@@ -270,17 +268,25 @@ error (int status, int errnum, const char *message, ...)
 # endif
 #endif
 }
+
+void
+error (int status, int errnum, const char *message, ...)
+{
+  va_list ap;
+  va_start (ap, message);
+  __error_internal (status, errnum, message, ap, 0);
+  va_end (ap);
+}
 
 /* Sometimes we want to have at most one error per line.  This
    variable controls whether this mode is selected or not.  */
 int error_one_per_line;
 
 void
-error_at_line (int status, int errnum, const char *file_name,
-	       unsigned int line_number, const char *message, ...)
+__error_at_line_internal (int status, int errnum, const char *file_name,
+			  unsigned int line_number, const char *message,
+			  va_list args, unsigned int mode_flags)
 {
-  va_list args;
-
   if (error_one_per_line)
     {
       static const char *old_file_name;
@@ -331,9 +337,7 @@ error_at_line (int status, int errnum, const char *file_name,
 	   file_name, line_number);
 #endif
 
-  va_start (args, message);
-  error_tail (status, errnum, message, args);
-  va_end (args);
+  error_tail (status, errnum, message, args, mode_flags);
 
 #ifdef _LIBC
   _IO_funlockfile (stderr);
@@ -343,6 +347,17 @@ error_at_line (int status, int errnum, const char *file_name,
 #endif
 }
 
+void
+error_at_line (int status, int errnum, const char *file_name,
+	       unsigned int line_number, const char *message, ...)
+{
+  va_list ap;
+  va_start (ap, message);
+  __error_at_line_internal (status, errnum, file_name, line_number,
+			    message, ap, 0);
+  va_end (ap);
+}
+
 #ifdef _LIBC
 /* Make the weak alias.  */
 # undef error
diff --git a/stdio-common/fxprintf.c b/stdio-common/fxprintf.c
index a028e8edd5..b784139175 100644
--- a/stdio-common/fxprintf.c
+++ b/stdio-common/fxprintf.c
@@ -24,10 +24,11 @@
 #include <libioP.h>
 
 static int
-locked_vfxprintf (FILE *fp, const char *fmt, va_list ap)
+locked_vfxprintf (FILE *fp, const char *fmt, va_list ap,
+		  unsigned int mode_flags)
 {
   if (_IO_fwide (fp, 0) <= 0)
-    return __vfprintf_internal (fp, fmt, ap, 0);
+    return __vfprintf_internal (fp, fmt, ap, mode_flags);
 
   /* We must convert the narrow format string to a wide one.
      Each byte can produce at most one wide character.  */
@@ -53,7 +54,7 @@ locked_vfxprintf (FILE *fp, const char *fmt, va_list ap)
   res = __mbsrtowcs (wfmt, &fmt, len, &mbstate);
 
   if (res != -1)
-    res = __vfwprintf_internal (fp, wfmt, ap, 0);
+    res = __vfwprintf_internal (fp, wfmt, ap, mode_flags);
 
   if (used_malloc)
     free (wfmt);
@@ -62,12 +63,13 @@ locked_vfxprintf (FILE *fp, const char *fmt, va_list ap)
 }
 
 int
-__vfxprintf (FILE *fp, const char *fmt, va_list ap)
+__vfxprintf (FILE *fp, const char *fmt, va_list ap,
+	     unsigned int mode_flags)
 {
   if (fp == NULL)
     fp = stderr;
   _IO_flockfile (fp);
-  int res = locked_vfxprintf (fp, fmt, ap);
+  int res = locked_vfxprintf (fp, fmt, ap, mode_flags);
   _IO_funlockfile (fp);
   return res;
 }
@@ -77,7 +79,7 @@ __fxprintf (FILE *fp, const char *fmt, ...)
 {
   va_list ap;
   va_start (ap, fmt);
-  int res = __vfxprintf (fp, fmt, ap);
+  int res = __vfxprintf (fp, fmt, ap, 0);
   va_end (ap);
   return res;
 }
@@ -94,7 +96,7 @@ __fxprintf_nocancel (FILE *fp, const char *fmt, ...)
   int save_flags2 = fp->_flags2;
   fp->_flags2 |= _IO_FLAGS2_NOTCANCEL;
 
-  int res = locked_vfxprintf (fp, fmt, ap);
+  int res = locked_vfxprintf (fp, fmt, ap, 0);
 
   fp->_flags2 = save_flags2;
   _IO_funlockfile (fp);
-- 
2.14.5