diff mbox series

[v2] stdio: Implement %#m for vfprintf and related functions

Message ID 87tuf7i4lk.fsf@oldenburg.str.redhat.com
State Committed
Headers
Series [v2] stdio: Implement %#m for vfprintf and related functions |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Florian Weimer Dec. 17, 2021, 4:22 p.m. UTC 
  %#m prints errno as an error constant if one is available, or
a decimal number as a fallback.  This intends to address the gap
that strerrorname_np does not work well with printf for unknown
error codes due to its NULL return values in those cases.

---
v2: Rebased on top of current trunk.

 NEWS                             |  3 ++
 manual/stdio.texi                |  8 +++-
 stdio-common/Makefile            |  4 +-
 stdio-common/tst-sprintf-errno.c | 93 ++++++++++++++++++++++++++++++++++++++++
 stdio-common/vfprintf-internal.c | 25 ++++++++---
 5 files changed, 125 insertions(+), 8 deletions(-)

Comments

Adhemerval Zanella Dec. 22, 2021, 3:33 p.m. UTC | #1 
On 17/12/2021 13:22, Florian Weimer via Libc-alpha wrote:
> %#m prints errno as an error constant if one is available, or
> a decimal number as a fallback.  This intends to address the gap
> that strerrorname_np does not work well with printf for unknown
> error codes due to its NULL return values in those cases.

CCing man-pages mantainer since they might want to update man-pages.

LGTM, just a minor style issue below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
> v2: Rebased on top of current trunk.
> 
>  NEWS                             |  3 ++
>  manual/stdio.texi                |  8 +++-
>  stdio-common/Makefile            |  4 +-
>  stdio-common/tst-sprintf-errno.c | 93 ++++++++++++++++++++++++++++++++++++++++
>  stdio-common/vfprintf-internal.c | 25 ++++++++---
>  5 files changed, 125 insertions(+), 8 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index c7200cd4e8..9294a7b953 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -99,6 +99,9 @@ Major new features:
>    is set to 'madvise' mode while the latter uses the system reserved
>    huge pages.
>  
> +* The printf family of functions now handles the flagged %#m conversion
> +  specifier, printing errno as an error constant (similar to strerrorname_np).
> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * On x86-64, the LD_PREFER_MAP_32BIT_EXEC environment variable support

Ok.

> diff --git a/manual/stdio.texi b/manual/stdio.texi
> index 29d01b9ec9..1fac8e5f78 100644
> --- a/manual/stdio.texi
> +++ b/manual/stdio.texi
> @@ -1949,6 +1949,9 @@ which can be parsed by the @code{strtoul} function (@pxref{Parsing of
>  Integers}) and @code{scanf} with the @samp{%i} conversion
>  (@pxref{Numeric Input Conversions}).
>  
> +For the @samp{%m} conversion, print an error constant or decimal error
> +number, instead of a (possibly translated) error message.
> +
>  @item @samp{'}
>  Separate the digits into groups as specified by the locale specified for
>  the @code{LC_NUMERIC} category; @pxref{General Numeric}.  This flag is a

Ok.

> @@ -2271,8 +2274,9 @@ is equivalent to:
>  fprintf (stderr, "can't open `%s': %s\n", filename, strerror (errno));
>  @end smallexample
>  
> -@noindent
> -The @samp{%m} conversion is a @glibcadj{} extension.
> +The @samp{%m} conversion can be used with the @samp{#} flag to print an
> +error constant, as provided by @code{strerrorname_np}.  Both @samp{%m}
> +and @samp{%#m} are @glibcadj{} extensions.
>  
>  The @samp{%p} conversion prints a pointer value.  The corresponding
>  argument must be of type @code{void *}.  In practice, you can use any

Ok.

> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index bbb3a2ca56..a69a296343 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -71,7 +71,9 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \
>  	 tst-printf-fp-free \
>  	 tst-printf-fp-leak \
>  	 test-strerr \
> -	 tst-printf-binary
> +	 tst-printf-binary \
> +         tst-sprintf-errno \

Indentation seems off here (although not wrong)

> +         # tests
>  
>  
>  test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble
> diff --git a/stdio-common/tst-sprintf-errno.c b/stdio-common/tst-sprintf-errno.c
> new file mode 100644
> index 0000000000..7622fdbc48
> --- /dev/null
> +++ b/stdio-common/tst-sprintf-errno.c
> @@ -0,0 +1,93 @@
> +/* Test the %m, %#m printf specifiers via asprintf.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <libc-diag.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +/* GCC does not yet know about the %#m specifier.  */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (11, "-Wformat=");
> +
> +static int
> +do_test (void)
> +{
> +  char buf[64];
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%m"), 16);
> +  TEST_COMPARE_STRING (buf, "Invalid argument");
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%#m"), 6);
> +  TEST_COMPARE_STRING (buf, "EINVAL");
> +
> +  errno = 0;
> +  TEST_COMPARE (sprintf (buf, "%m"), 7);
> +  TEST_COMPARE_STRING (buf, "Success");
> +
> +  errno = 0;
> +  TEST_COMPARE (sprintf (buf, "%#m"), 1);
> +  TEST_COMPARE_STRING (buf, "0");
> +
> +  errno = -1;
> +  TEST_COMPARE (sprintf (buf, "%m"), 16);
> +  TEST_COMPARE_STRING (buf, "Unknown error -1");
> +
> +  errno = -1;
> +  TEST_COMPARE (sprintf (buf, "%#m"), 2);
> +  TEST_COMPARE_STRING (buf, "-1");
> +
> +  errno = 1002003;
> +  TEST_COMPARE (sprintf (buf, "%m"), 21);
> +  TEST_COMPARE_STRING (buf, "Unknown error 1002003");
> +
> +  errno = 1002003;
> +  TEST_COMPARE (sprintf (buf, "%#m"), 7);
> +  TEST_COMPARE_STRING (buf, "1002003");
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%20m"), 20);
> +  TEST_COMPARE_STRING (buf, "    Invalid argument");
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%#20m"), 20);
> +  TEST_COMPARE_STRING (buf, "              EINVAL");
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%-20m"), 20);
> +  TEST_COMPARE_STRING (buf, "Invalid argument    ");
> +
> +  errno = EINVAL;
> +  TEST_COMPARE (sprintf (buf, "%-#20m"), 20);
> +  TEST_COMPARE_STRING (buf, "EINVAL              ");
> +
> +  errno = 0;
> +  TEST_COMPARE (sprintf (buf, "%-20m"), 20);
> +  TEST_COMPARE_STRING (buf, "Success             ");
> +
> +  errno = 0;
> +  TEST_COMPARE (sprintf (buf, "%-#20m"), 20);
> +  TEST_COMPARE_STRING (buf, "0                   ");
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index e717f50073..8547090d45 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -950,11 +950,26 @@ static const uint8_t jump_table[] =
>  									      \
>      LABEL (form_strerror):						      \
>        /* Print description of error ERRNO.  */				      \
> -      string =								      \
> -	(CHAR_T *) __strerror_r (save_errno, (char *) work_buffer,	      \
> -				 WORK_BUFFER_SIZE * sizeof (CHAR_T));	      \
> -      is_long = 0;		/* This is no wide-char string.  */	      \
> -      goto LABEL (print_string)
> +      if (alt)								      \
> +	string = (CHAR_T *) __get_errname (save_errno);			      \
> +      else								      \
> +	string = (CHAR_T *) __strerror_r (save_errno, (char *) work_buffer,   \
> +					  WORK_BUFFER_SIZE * sizeof (CHAR_T));\
> +      if (string == NULL)						\
> +	{								      \
> +          /* Print as a decimal number. */				      \
> +          base = 10;							      \
> +	  is_negative = save_errno < 0;					      \
> +	  number.word = save_errno;					      \
> +	  if (is_negative)						      \
> +	    number.word = -number.word;					      \
> +	  goto LABEL (number);						      \
> +	}								      \
> +      else								      \
> +	{								      \
> +	  is_long = 0;	/* This is no wide-char string.  */		      \
> +	  goto LABEL (print_string);					      \
> +	}
>  
>  #ifdef COMPILE_WPRINTF
>  # define process_string_arg()						      \
> 

Ok.
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index c7200cd4e8..9294a7b953 100644
--- a/NEWS
+++ b/NEWS
@@ -99,6 +99,9 @@  Major new features:
   is set to 'madvise' mode while the latter uses the system reserved
   huge pages.
 
+* The printf family of functions now handles the flagged %#m conversion
+  specifier, printing errno as an error constant (similar to strerrorname_np).
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * On x86-64, the LD_PREFER_MAP_32BIT_EXEC environment variable support
diff --git a/manual/stdio.texi b/manual/stdio.texi
index 29d01b9ec9..1fac8e5f78 100644
--- a/manual/stdio.texi
+++ b/manual/stdio.texi
@@ -1949,6 +1949,9 @@  which can be parsed by the @code{strtoul} function (@pxref{Parsing of
 Integers}) and @code{scanf} with the @samp{%i} conversion
 (@pxref{Numeric Input Conversions}).
 
+For the @samp{%m} conversion, print an error constant or decimal error
+number, instead of a (possibly translated) error message.
+
 @item @samp{'}
 Separate the digits into groups as specified by the locale specified for
 the @code{LC_NUMERIC} category; @pxref{General Numeric}.  This flag is a
@@ -2271,8 +2274,9 @@  is equivalent to:
 fprintf (stderr, "can't open `%s': %s\n", filename, strerror (errno));
 @end smallexample
 
-@noindent
-The @samp{%m} conversion is a @glibcadj{} extension.
+The @samp{%m} conversion can be used with the @samp{#} flag to print an
+error constant, as provided by @code{strerrorname_np}.  Both @samp{%m}
+and @samp{%#m} are @glibcadj{} extensions.
 
 The @samp{%p} conversion prints a pointer value.  The corresponding
 argument must be of type @code{void *}.  In practice, you can use any
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index bbb3a2ca56..a69a296343 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -71,7 +71,9 @@  tests := tstscanf test_rdwr test-popen tstgetln test-fseek \
 	 tst-printf-fp-free \
 	 tst-printf-fp-leak \
 	 test-strerr \
-	 tst-printf-binary
+	 tst-printf-binary \
+         tst-sprintf-errno \
+         # tests
 
 
 test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble
diff --git a/stdio-common/tst-sprintf-errno.c b/stdio-common/tst-sprintf-errno.c
new file mode 100644
index 0000000000..7622fdbc48
--- /dev/null
+++ b/stdio-common/tst-sprintf-errno.c
@@ -0,0 +1,93 @@ 
+/* Test the %m, %#m printf specifiers via asprintf.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <libc-diag.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/support.h>
+
+/* GCC does not yet know about the %#m specifier.  */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (11, "-Wformat=");
+
+static int
+do_test (void)
+{
+  char buf[64];
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%m"), 16);
+  TEST_COMPARE_STRING (buf, "Invalid argument");
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%#m"), 6);
+  TEST_COMPARE_STRING (buf, "EINVAL");
+
+  errno = 0;
+  TEST_COMPARE (sprintf (buf, "%m"), 7);
+  TEST_COMPARE_STRING (buf, "Success");
+
+  errno = 0;
+  TEST_COMPARE (sprintf (buf, "%#m"), 1);
+  TEST_COMPARE_STRING (buf, "0");
+
+  errno = -1;
+  TEST_COMPARE (sprintf (buf, "%m"), 16);
+  TEST_COMPARE_STRING (buf, "Unknown error -1");
+
+  errno = -1;
+  TEST_COMPARE (sprintf (buf, "%#m"), 2);
+  TEST_COMPARE_STRING (buf, "-1");
+
+  errno = 1002003;
+  TEST_COMPARE (sprintf (buf, "%m"), 21);
+  TEST_COMPARE_STRING (buf, "Unknown error 1002003");
+
+  errno = 1002003;
+  TEST_COMPARE (sprintf (buf, "%#m"), 7);
+  TEST_COMPARE_STRING (buf, "1002003");
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%20m"), 20);
+  TEST_COMPARE_STRING (buf, "    Invalid argument");
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%#20m"), 20);
+  TEST_COMPARE_STRING (buf, "              EINVAL");
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%-20m"), 20);
+  TEST_COMPARE_STRING (buf, "Invalid argument    ");
+
+  errno = EINVAL;
+  TEST_COMPARE (sprintf (buf, "%-#20m"), 20);
+  TEST_COMPARE_STRING (buf, "EINVAL              ");
+
+  errno = 0;
+  TEST_COMPARE (sprintf (buf, "%-20m"), 20);
+  TEST_COMPARE_STRING (buf, "Success             ");
+
+  errno = 0;
+  TEST_COMPARE (sprintf (buf, "%-#20m"), 20);
+  TEST_COMPARE_STRING (buf, "0                   ");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index e717f50073..8547090d45 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -950,11 +950,26 @@  static const uint8_t jump_table[] =
 									      \
     LABEL (form_strerror):						      \
       /* Print description of error ERRNO.  */				      \
-      string =								      \
-	(CHAR_T *) __strerror_r (save_errno, (char *) work_buffer,	      \
-				 WORK_BUFFER_SIZE * sizeof (CHAR_T));	      \
-      is_long = 0;		/* This is no wide-char string.  */	      \
-      goto LABEL (print_string)
+      if (alt)								      \
+	string = (CHAR_T *) __get_errname (save_errno);			      \
+      else								      \
+	string = (CHAR_T *) __strerror_r (save_errno, (char *) work_buffer,   \
+					  WORK_BUFFER_SIZE * sizeof (CHAR_T));\
+      if (string == NULL)						\
+	{								      \
+          /* Print as a decimal number. */				      \
+          base = 10;							      \
+	  is_negative = save_errno < 0;					      \
+	  number.word = save_errno;					      \
+	  if (is_negative)						      \
+	    number.word = -number.word;					      \
+	  goto LABEL (number);						      \
+	}								      \
+      else								      \
+	{								      \
+	  is_long = 0;	/* This is no wide-char string.  */		      \
+	  goto LABEL (print_string);					      \
+	}
 
 #ifdef COMPILE_WPRINTF
 # define process_string_arg()						      \