public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] vfprintf cleanups to avoid -Waddress warning
@ 2021-09-22  7:09 Florian Weimer
  2021-09-22  7:10 ` [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro Florian Weimer
  2021-09-22  7:10 ` [PATCH 2/2] vfprintf: Unify argument handling in process_arg Florian Weimer
  0 siblings, 2 replies; 6+ messages in thread
From: Florian Weimer @ 2021-09-22  7:09 UTC (permalink / raw)
  To: libc-alpha; +Cc: Martin Sebor

These two patches slightly clean up stdio-common/vfprintf-internal.c,
reducing code duplication slightly and removing a GCC 12 warning.

Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
build-many-glibcs.py.

Thanks,
Florian

Florian Weimer (2):
  vfprintf: Handle floating-point cases outside of process_arg macro
  vfprintf: Unify argument handling in process_arg

 stdio-common/vfprintf-internal.c | 392 +++++++++++++------------------
 1 file changed, 164 insertions(+), 228 deletions(-)


base-commit: 1356f38df5be0776823eb2c40cc4e607b86b9680
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro
  2021-09-22  7:09 [PATCH 0/2] vfprintf cleanups to avoid -Waddress warning Florian Weimer
@ 2021-09-22  7:10 ` Florian Weimer
  2021-09-22 13:55   ` Andreas Schwab
  2021-09-22  7:10 ` [PATCH 2/2] vfprintf: Unify argument handling in process_arg Florian Weimer
  1 sibling, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2021-09-22  7:10 UTC (permalink / raw)
  To: libc-alpha

A lot of the code is unique to the positional and non-positional
code.  Also unify the decimal and hexadecimal cases via the new
helper function __printf_fp_spec.
---
 stdio-common/vfprintf-internal.c | 186 +++++++++++++------------------
 1 file changed, 75 insertions(+), 111 deletions(-)

diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index 3f3d1e148a..8e31a964d1 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -342,6 +342,18 @@ outstring_converted_wide_string (FILE *s, const OTHER_CHAR_T *src, int prec,
   return done;
 }
 
+/* Calls __printf_fp or __printf_fphex based on the value of the
+   format specifier INFO->spec.  */
+static inline int
+__printf_fp_spec (FILE *fp, const struct printf_info *info,
+		  const void *const *args)
+{
+  if (info->spec == 'a' || info->spec == 'A')
+    return __printf_fphex (fp, info, args);
+  else
+    return __printf_fp (fp, info, args);
+}
+
 /* For handling long_double and longlong we use the same flag.  If
    `long' and `long long' are effectively the same type define it to
    zero.  */
@@ -902,116 +914,6 @@ static const uint8_t jump_table[] =
 	  break;							      \
 	}								      \
 									      \
-    LABEL (form_float):							      \
-      {									      \
-	/* Floating-point number.  This is handled by printf_fp.c.  */	      \
-	const void *ptr;						      \
-	int function_done;						      \
-									      \
-	if (fspec == NULL)						      \
-	  {								      \
-	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
-	      is_long_double = 0;					      \
-									      \
-	    struct printf_info info = { .prec = prec,			      \
-					.width = width,			      \
-					.spec = spec,			      \
-					.is_long_double = is_long_double,     \
-					.is_short = is_short,		      \
-					.is_long = is_long,		      \
-					.alt = alt,			      \
-					.space = space,			      \
-					.left = left,			      \
-					.showsign = showsign,		      \
-					.group = group,			      \
-					.pad = pad,			      \
-					.extra = 0,			      \
-					.i18n = use_outdigits,		      \
-					.wide = sizeof (CHAR_T) != 1,	      \
-					.is_binary128 = 0};		      \
-									      \
-	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
-	    ptr = (const void *) &the_arg;				      \
-									      \
-	    function_done = __printf_fp (s, &info, &ptr);		      \
-	  }								      \
-	else								      \
-	  {								      \
-	    ptr = (const void *) &args_value[fspec->data_arg];		      \
-	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
-	      {								      \
-		fspec->data_arg_type = PA_DOUBLE;			      \
-		fspec->info.is_long_double = 0;				      \
-	      }								      \
-	    SETUP_FLOAT128_INFO (fspec->info);				      \
-									      \
-	    function_done = __printf_fp (s, &fspec->info, &ptr);	      \
-	  }								      \
-									      \
-	if (function_done < 0)						      \
-	  {								      \
-	    /* Error in print handler; up to handler to set errno.  */	      \
-	    done = -1;							      \
-	    goto all_done;						      \
-	  }								      \
-									      \
-	done_add (function_done);					      \
-      }									      \
-      break;								      \
-									      \
-    LABEL (form_floathex):						      \
-      {									      \
-	/* Floating point number printed as hexadecimal number.  */	      \
-	const void *ptr;						      \
-	int function_done;						      \
-									      \
-	if (fspec == NULL)						      \
-	  {								      \
-	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
-	      is_long_double = 0;					      \
-									      \
-	    struct printf_info info = { .prec = prec,			      \
-					.width = width,			      \
-					.spec = spec,			      \
-					.is_long_double = is_long_double,     \
-					.is_short = is_short,		      \
-					.is_long = is_long,		      \
-					.alt = alt,			      \
-					.space = space,			      \
-					.left = left,			      \
-					.showsign = showsign,		      \
-					.group = group,			      \
-					.pad = pad,			      \
-					.extra = 0,			      \
-					.wide = sizeof (CHAR_T) != 1,	      \
-					.is_binary128 = 0};		      \
-									      \
-	    PARSE_FLOAT_VA_ARG_EXTENDED (info);				      \
-	    ptr = (const void *) &the_arg;				      \
-									      \
-	    function_done = __printf_fphex (s, &info, &ptr);		      \
-	  }								      \
-	else								      \
-	  {								      \
-	    ptr = (const void *) &args_value[fspec->data_arg];		      \
-	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))    \
-	      fspec->info.is_long_double = 0;				      \
-	    SETUP_FLOAT128_INFO (fspec->info);				      \
-									      \
-	    function_done = __printf_fphex (s, &fspec->info, &ptr);	      \
-	  }								      \
-									      \
-	if (function_done < 0)						      \
-	  {								      \
-	    /* Error in print handler; up to handler to set errno.  */	      \
-	    done = -1;							      \
-	    goto all_done;						      \
-	  }								      \
-									      \
-	done_add (function_done);					      \
-      }									      \
-      break;								      \
-									      \
     LABEL (form_pointer):						      \
       /* Generic pointer.  */						      \
       {									      \
@@ -1646,6 +1548,45 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
 	  process_arg (((struct printf_spec *) NULL));
 	  process_string_arg (((struct printf_spec *) NULL));
 
+	LABEL (form_float):
+	LABEL (form_floathex):
+	  {
+	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))
+	      is_long_double = 0;
+
+	    struct printf_info info =
+	      {
+		.prec = prec,
+		.width = width,
+		.spec = spec,
+		.is_long_double = is_long_double,
+		.is_short = is_short,
+		.is_long = is_long,
+		.alt = alt,
+		.space = space,
+		.left = left,
+		.showsign = showsign,
+		.group = group,
+		.pad = pad,
+		.extra = 0,
+		.i18n = use_outdigits,
+		.wide = sizeof (CHAR_T) != 1,
+		.is_binary128 = 0
+	      };
+
+	    PARSE_FLOAT_VA_ARG_EXTENDED (info);
+	    const void *ptr = &the_arg;
+
+	    int function_done = __printf_fp_spec (s, &info, &ptr);
+	    if (function_done < 0)
+	      {
+		done = -1;
+		goto all_done;
+	      }
+	    done_add (function_done);
+	  }
+	  break;
+
 	LABEL (form_unknown):
 	  if (spec == L_('\0'))
 	    {
@@ -1901,7 +1842,6 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 	unsigned long int word;
       } number;
       int base;
-      union printf_arg the_arg;
       CHAR_T *string;		/* Pointer to argument string.  */
 
       /* Fill variables from values in struct.  */
@@ -1996,6 +1936,30 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 	  process_arg ((&specs[nspecs_done]));
 	  process_string_arg ((&specs[nspecs_done]));
 
+	  LABEL (form_float):
+	  LABEL (form_floathex):
+	  {
+	    const void *ptr
+	      = (const void *) &args_value[specs[nspecs_done].data_arg];
+	    if (__glibc_unlikely ((mode_flags & PRINTF_LDBL_IS_DBL) != 0))
+	      {
+		specs[nspecs_done].data_arg_type = PA_DOUBLE;
+		specs[nspecs_done].info.is_long_double = 0;
+	      }
+	    SETUP_FLOAT128_INFO (specs[nspecs_done].info);
+
+	    int function_done
+	      = __printf_fp_spec (s, &specs[nspecs_done].info, &ptr);
+	    if (function_done < 0)
+	      {
+		/* Error in print handler; up to handler to set errno.  */
+		done = -1;
+		goto all_done;
+	      }
+	    done_add (function_done);
+	  }
+	  break;
+
 	  LABEL (form_unknown):
 	  {
 	    unsigned int i;
-- 
2.31.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] vfprintf: Unify argument handling in process_arg
  2021-09-22  7:09 [PATCH 0/2] vfprintf cleanups to avoid -Waddress warning Florian Weimer
  2021-09-22  7:10 ` [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro Florian Weimer
@ 2021-09-22  7:10 ` Florian Weimer
  2021-09-22 14:17   ` Andreas Schwab
  1 sibling, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2021-09-22  7:10 UTC (permalink / raw)
  To: libc-alpha

Instead of checking a pointer argument for NULL, use helper macros
defined differently in the non-positional and positional cases.
This avoids frequent conditional checks and a GCC 12 warning
about comparing pointers against NULL which cannot be NULL.
---
 stdio-common/vfprintf-internal.c | 206 +++++++++++++------------------
 1 file changed, 89 insertions(+), 117 deletions(-)

diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index 8e31a964d1..7a6aac2a14 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -650,8 +650,9 @@ static const uint8_t jump_table[] =
       REF (form_unknown)        /* for 'I' */				      \
     }
 
-
-#define process_arg(fspec)						      \
+/* Before invoking this macro, PROCESS_ARG_INT etc. macros have to be
+   defined to extract one argument of the appropriate type.  */
+#define process_arg()						              \
       /* Start real work.  We know about all flags and modifiers and	      \
 	 now process the wanted format specifier.  */			      \
     LABEL (form_percent):						      \
@@ -665,13 +666,7 @@ static const uint8_t jump_table[] =
 									      \
       if (is_longlong)							      \
 	{								      \
-	  long long int signed_number;					      \
-									      \
-	  if (fspec == NULL)						      \
-	    signed_number = va_arg (ap, long long int);			      \
-	  else								      \
-	    signed_number = args_value[fspec->data_arg].pa_long_long_int;     \
-									      \
+	  long long int signed_number = PROCESS_ARG_LONG_LONG_INT ();	      \
 	  is_negative = signed_number < 0;				      \
 	  number.longlong = is_negative ? (- signed_number) : signed_number;  \
 									      \
@@ -680,29 +675,14 @@ static const uint8_t jump_table[] =
       else								      \
 	{								      \
 	  long int signed_number;					      \
-									      \
-	  if (fspec == NULL)						      \
-	    {								      \
-	      if (is_long_num)						      \
-		signed_number = va_arg (ap, long int);			      \
-	      else if (is_char)						      \
-		signed_number = (signed char) va_arg (ap, unsigned int);      \
-	      else if (!is_short)					      \
-		signed_number = va_arg (ap, int);			      \
-	      else							      \
-		signed_number = (short int) va_arg (ap, unsigned int);	      \
-	    }								      \
+	  if (is_long_num)						      \
+	    signed_number = PROCESS_ARG_LONG_INT ();			      \
+	  else if (is_char)						      \
+	    signed_number = (signed char) PROCESS_ARG_UNSIGNED_INT ();	      \
+	  else if (!is_short)						      \
+	    signed_number = PROCESS_ARG_INT ();				      \
 	  else								      \
-	    if (is_long_num)						      \
-	      signed_number = args_value[fspec->data_arg].pa_long_int;	      \
-	    else if (is_char)						      \
-	      signed_number = (signed char)				      \
-		args_value[fspec->data_arg].pa_u_int;			      \
-	    else if (!is_short)						      \
-	      signed_number = args_value[fspec->data_arg].pa_int;	      \
-	    else							      \
-	      signed_number = (short int)				      \
-		args_value[fspec->data_arg].pa_u_int;			      \
+	    signed_number = (short int) PROCESS_ARG_UNSIGNED_INT ();	      \
 									      \
 	  is_negative = signed_number < 0;				      \
 	  number.word = is_negative ? (- signed_number) : signed_number;      \
@@ -737,10 +717,7 @@ static const uint8_t jump_table[] =
 									      \
       if (is_longlong)							      \
 	{								      \
-	  if (fspec == NULL)						      \
-	    number.longlong = va_arg (ap, unsigned long long int);	      \
-	  else								      \
-	    number.longlong = args_value[fspec->data_arg].pa_u_long_long_int; \
+	  number.longlong = PROCESS_ARG_UNSIGNED_LONG_LONG_INT ();	      \
 									      \
 	LABEL (longlong_number):					      \
 	  if (prec < 0)							      \
@@ -776,28 +753,14 @@ static const uint8_t jump_table[] =
 	}								      \
       else								      \
 	{								      \
-	  if (fspec == NULL)						      \
-	    {								      \
-	      if (is_long_num)						      \
-		number.word = va_arg (ap, unsigned long int);		      \
-	      else if (is_char)						      \
-		number.word = (unsigned char) va_arg (ap, unsigned int);      \
-	      else if (!is_short)					      \
-		number.word = va_arg (ap, unsigned int);		      \
-	      else							      \
-		number.word = (unsigned short int) va_arg (ap, unsigned int); \
-	    }								      \
+	  if (is_long_num)						      \
+	    number.word = PROCESS_ARG_UNSIGNED_LONG_INT ();		      \
+	  else if (is_char)						      \
+	    number.word = (unsigned char) PROCESS_ARG_UNSIGNED_INT ();	      \
+	  else if (!is_short)						      \
+	    number.word = PROCESS_ARG_UNSIGNED_INT ();			      \
 	  else								      \
-	    if (is_long_num)						      \
-	      number.word = args_value[fspec->data_arg].pa_u_long_int;	      \
-	    else if (is_char)						      \
-	      number.word = (unsigned char)				      \
-		args_value[fspec->data_arg].pa_u_int;			      \
-	    else if (!is_short)						      \
-	      number.word = args_value[fspec->data_arg].pa_u_int;	      \
-	    else							      \
-	      number.word = (unsigned short int)			      \
-		args_value[fspec->data_arg].pa_u_int;			      \
+	    number.word = (unsigned short int) PROCESS_ARG_UNSIGNED_INT ();   \
 									      \
 	LABEL (number):							      \
 	  if (prec < 0)							      \
@@ -917,11 +880,7 @@ static const uint8_t jump_table[] =
     LABEL (form_pointer):						      \
       /* Generic pointer.  */						      \
       {									      \
-	const void *ptr;						      \
-	if (fspec == NULL)						      \
-	  ptr = va_arg (ap, void *);					      \
-	else								      \
-	  ptr = args_value[fspec->data_arg].pa_pointer;			      \
+	const void *ptr = PROCESS_ARG_POINTER ();			      \
 	if (ptr != NULL)						      \
 	  {								      \
 	    /* If the pointer is not NULL, write it as a %#x spec.  */	      \
@@ -962,30 +921,17 @@ static const uint8_t jump_table[] =
 	    __libc_fatal ("*** %n in writable segment detected ***\n");	      \
 	}								      \
       /* Answer the count of characters written.  */			      \
-      if (fspec == NULL)						      \
-	{								      \
-	  if (is_longlong)						      \
-	    *(long long int *) va_arg (ap, void *) = done;		      \
-	  else if (is_long_num)						      \
-	    *(long int *) va_arg (ap, void *) = done;			      \
-	  else if (is_char)						      \
-	    *(char *) va_arg (ap, void *) = done;			      \
-	  else if (!is_short)						      \
-	    *(int *) va_arg (ap, void *) = done;			      \
-	  else								      \
-	    *(short int *) va_arg (ap, void *) = done;			      \
-	}								      \
+      void *ptrptr = PROCESS_ARG_POINTER ();				      \
+      if (is_longlong)							      \
+	*(long long int *) ptrptr = done;				      \
+      else if (is_long_num)						      \
+	*(long int *) ptrptr = done;					      \
+      else if (is_char)							      \
+	*(char *) ptrptr = done;					      \
+      else if (!is_short)						      \
+	*(int *) ptrptr = done;						      \
       else								      \
-	if (is_longlong)						      \
-	  *(long long int *) args_value[fspec->data_arg].pa_pointer = done;   \
-	else if (is_long_num)						      \
-	  *(long int *) args_value[fspec->data_arg].pa_pointer = done;	      \
-	else if (is_char)						      \
-	  *(char *) args_value[fspec->data_arg].pa_pointer = done;	      \
-	else if (!is_short)						      \
-	  *(int *) args_value[fspec->data_arg].pa_pointer = done;	      \
-	else								      \
-	  *(short int *) args_value[fspec->data_arg].pa_pointer = done;	      \
+	*(short int *) ptrptr = done;					      \
       break;								      \
 									      \
     LABEL (form_strerror):						      \
@@ -997,7 +943,7 @@ static const uint8_t jump_table[] =
       goto LABEL (print_string)
 
 #ifdef COMPILE_WPRINTF
-# define process_string_arg(fspec) \
+# define process_string_arg()						      \
     LABEL (form_character):						      \
       /* Character.  */							      \
       if (is_long)							      \
@@ -1005,11 +951,7 @@ static const uint8_t jump_table[] =
       --width;	/* Account for the character itself.  */		      \
       if (!left)							      \
 	PAD (L' ');							      \
-      if (fspec == NULL)						      \
-	outchar (__btowc ((unsigned char) va_arg (ap, int))); /* Promoted. */ \
-      else								      \
-	outchar (__btowc ((unsigned char)				      \
-			  args_value[fspec->data_arg].pa_int));		      \
+      outchar (__btowc ((unsigned char) PROCESS_ARG_INT ())); /* Promoted. */ \
       if (left)								      \
 	PAD (L' ');							      \
       break;								      \
@@ -1020,10 +962,7 @@ static const uint8_t jump_table[] =
 	--width;							      \
 	if (!left)							      \
 	  PAD (L' ');							      \
-	if (fspec == NULL)						      \
-	  outchar (va_arg (ap, wchar_t));				      \
-	else								      \
-	  outchar (args_value[fspec->data_arg].pa_wchar);		      \
+	outchar (PROCESS_ARG_WCHAR_T ());				      \
 	if (left)							      \
 	  PAD (L' ');							      \
       }									      \
@@ -1035,10 +974,7 @@ static const uint8_t jump_table[] =
 									      \
 	/* The string argument could in fact be `char *' or `wchar_t *'.      \
 	   But this should not make a difference here.  */		      \
-	if (fspec == NULL)						      \
-	  string = (CHAR_T *) va_arg (ap, const wchar_t *);		      \
-	else								      \
-	  string = (CHAR_T *) args_value[fspec->data_arg].pa_wstring;	      \
+	string = (CHAR_T *) PROCESS_ARG_WSTRING ();			      \
 									      \
 	/* Entry point for printing other strings.  */			      \
       LABEL (print_string):						      \
@@ -1090,7 +1026,7 @@ static const uint8_t jump_table[] =
       }									      \
       break;
 #else
-# define process_string_arg(fspec) \
+# define process_string_arg()						      \
     LABEL (form_character):						      \
       /* Character.  */							      \
       if (is_long)							      \
@@ -1098,10 +1034,7 @@ static const uint8_t jump_table[] =
       --width;	/* Account for the character itself.  */		      \
       if (!left)							      \
 	PAD (' ');							      \
-      if (fspec == NULL)						      \
-	outchar ((unsigned char) va_arg (ap, int)); /* Promoted.  */	      \
-      else								      \
-	outchar ((unsigned char) args_value[fspec->data_arg].pa_int);	      \
+      outchar ((unsigned char) PROCESS_ARG_INT ()); /* Promoted.  */	      \
       if (left)								      \
 	PAD (' ');							      \
       break;								      \
@@ -1114,9 +1047,7 @@ static const uint8_t jump_table[] =
 	size_t len;							      \
 									      \
 	memset (&mbstate, '\0', sizeof (mbstate_t));			      \
-	len = __wcrtomb (buf, (fspec == NULL ? va_arg (ap, wchar_t)	      \
-			       : args_value[fspec->data_arg].pa_wchar),	      \
-			 &mbstate);					      \
+	len = __wcrtomb (buf, PROCESS_ARG_WCHAR_T (), &mbstate);	      \
 	if (len == (size_t) -1)						      \
 	  {								      \
 	    /* Something went wrong during the conversion.  Bail out.  */     \
@@ -1138,10 +1069,7 @@ static const uint8_t jump_table[] =
 									      \
 	/* The string argument could in fact be `char *' or `wchar_t *'.      \
 	   But this should not make a difference here.  */		      \
-	if (fspec == NULL)						      \
-	  string = (char *) va_arg (ap, const char *);			      \
-	else								      \
-	  string = (char *) args_value[fspec->data_arg].pa_string;	      \
+	string = (char *) PROCESS_ARG_STRING ();			      \
 									      \
 	/* Entry point for printing other strings.  */			      \
       LABEL (print_string):						      \
@@ -1322,7 +1250,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
       STEP0_3_TABLE;
       STEP4_TABLE;
 
-      union printf_arg *args_value;	/* This is not used here but ... */
       int is_negative;	/* Flag for negative number.  */
       union
       {
@@ -1337,7 +1264,9 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
       int left = 0;	/* Left-justify output.  */
       int showsign = 0;	/* Always begin with plus or minus sign.  */
       int group = 0;	/* Print numbers according grouping rules.  */
-      int is_long_double = 0; /* Argument is long double/ long long int.  */
+      /* Argument is long double/long long int.  Only used if
+	 double/long double or long int/long long int are distinct.  */
+      int is_long_double __attribute__ ((unused)) = 0;
       int is_short = 0;	/* Argument is short int.  */
       int is_long = 0;	/* Argument is long int.  */
       int is_char = 0;	/* Argument is promoted (unsigned) char.  */
@@ -1545,8 +1474,28 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
       /* Process current format.  */
       while (1)
 	{
-	  process_arg (((struct printf_spec *) NULL));
-	  process_string_arg (((struct printf_spec *) NULL));
+#define PROCESS_ARG_INT() va_arg (ap, int)
+#define PROCESS_ARG_LONG_INT() va_arg (ap, long int)
+#define PROCESS_ARG_LONG_LONG_INT() va_arg (ap, long long int)
+#define PROCESS_ARG_POINTER() va_arg (ap, void *)
+#define PROCESS_ARG_STRING() va_arg (ap, const char *)
+#define PROCESS_ARG_UNSIGNED_INT() va_arg (ap, unsigned int)
+#define PROCESS_ARG_UNSIGNED_LONG_INT() va_arg (ap, unsigned long int)
+#define PROCESS_ARG_UNSIGNED_LONG_LONG_INT() va_arg (ap, unsigned long long int)
+#define PROCESS_ARG_WCHAR_T() va_arg (ap, wchar_t)
+#define PROCESS_ARG_WSTRING() va_arg (ap, const wchar_t *)
+	  process_arg ();
+	  process_string_arg ();
+#undef PROCESS_ARG_INT
+#undef PROCESS_ARG_LONG_INT
+#undef PROCESS_ARG_LONG_LONG_INT
+#undef PROCESS_ARG_POINTER
+#undef PROCESS_ARG_STRING
+#undef PROCESS_ARG_UNSIGNED_INT
+#undef PROCESS_ARG_UNSIGNED_LONG_INT
+#undef PROCESS_ARG_UNSIGNED_LONG_LONG_INT
+#undef PROCESS_ARG_WCHAR_T
+#undef PROCESS_ARG_WSTRING
 
 	LABEL (form_float):
 	LABEL (form_floathex):
@@ -1850,7 +1799,8 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
       int left = specs[nspecs_done].info.left;
       int showsign = specs[nspecs_done].info.showsign;
       int group = specs[nspecs_done].info.group;
-      int is_long_double = specs[nspecs_done].info.is_long_double;
+      int is_long_double __attribute__ ((unused))
+	= specs[nspecs_done].info.is_long_double;
       int is_short = specs[nspecs_done].info.is_short;
       int is_char = specs[nspecs_done].info.is_char;
       int is_long = specs[nspecs_done].info.is_long;
@@ -1933,8 +1883,30 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
 
 	  JUMP (spec, step4_jumps);
 
-	  process_arg ((&specs[nspecs_done]));
-	  process_string_arg ((&specs[nspecs_done]));
+#define PROCESS_ARG_DATA args_value[specs[nspecs_done].data_arg]
+#define PROCESS_ARG_INT() PROCESS_ARG_DATA.pa_int
+#define PROCESS_ARG_LONG_INT() PROCESS_ARG_DATA.pa_long_int
+#define PROCESS_ARG_LONG_LONG_INT() PROCESS_ARG_DATA.pa_long_long_int
+#define PROCESS_ARG_POINTER() PROCESS_ARG_DATA.pa_pointer
+#define PROCESS_ARG_STRING() PROCESS_ARG_DATA.pa_string
+#define PROCESS_ARG_UNSIGNED_INT() PROCESS_ARG_DATA.pa_u_int
+#define PROCESS_ARG_UNSIGNED_LONG_INT() PROCESS_ARG_DATA.pa_u_long_int
+#define PROCESS_ARG_UNSIGNED_LONG_LONG_INT() PROCESS_ARG_DATA.pa_u_long_long_int
+#define PROCESS_ARG_WCHAR_T() PROCESS_ARG_DATA.pa_wchar
+#define PROCESS_ARG_WSTRING() PROCESS_ARG_DATA.pa_wstring
+	  process_arg ();
+	  process_string_arg ();
+#undef PROCESS_ARG_DATA
+#undef PROCESS_ARG_INT
+#undef PROCESS_ARG_LONG_INT
+#undef PROCESS_ARG_LONG_LONG_INT
+#undef PROCESS_ARG_POINTER
+#undef PROCESS_ARG_STRING
+#undef PROCESS_ARG_UNSIGNED_INT
+#undef PROCESS_ARG_UNSIGNED_LONG_INT
+#undef PROCESS_ARG_UNSIGNED_LONG_LONG_INT
+#undef PROCESS_ARG_WCHAR_T
+#undef PROCESS_ARG_WSTRING
 
 	  LABEL (form_float):
 	  LABEL (form_floathex):
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro
  2021-09-22  7:10 ` [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro Florian Weimer
@ 2021-09-22 13:55   ` Andreas Schwab
  0 siblings, 0 replies; 6+ messages in thread
From: Andreas Schwab @ 2021-09-22 13:55 UTC (permalink / raw)
  To: Florian Weimer via Libc-alpha; +Cc: Florian Weimer

On Sep 22 2021, Florian Weimer via Libc-alpha wrote:

> A lot of the code is unique to the positional and non-positional
> code.  Also unify the decimal and hexadecimal cases via the new
> helper function __printf_fp_spec.

Ok.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] vfprintf: Unify argument handling in process_arg
  2021-09-22  7:10 ` [PATCH 2/2] vfprintf: Unify argument handling in process_arg Florian Weimer
@ 2021-09-22 14:17   ` Andreas Schwab
  2021-09-23 10:07     ` Florian Weimer
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2021-09-22 14:17 UTC (permalink / raw)
  To: Florian Weimer via Libc-alpha; +Cc: Florian Weimer

On Sep 22 2021, Florian Weimer via Libc-alpha wrote:

> Instead of checking a pointer argument for NULL, use helper macros
> defined differently in the non-positional and positional cases.
> This avoids frequent conditional checks and a GCC 12 warning
> about comparing pointers against NULL which cannot be NULL.

Ok.

> @@ -1545,8 +1474,28 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
>        /* Process current format.  */
>        while (1)
>  	{
> -	  process_arg (((struct printf_spec *) NULL));
> -	  process_string_arg (((struct printf_spec *) NULL));
> +#define PROCESS_ARG_INT() va_arg (ap, int)
> +#define PROCESS_ARG_LONG_INT() va_arg (ap, long int)
> +#define PROCESS_ARG_LONG_LONG_INT() va_arg (ap, long long int)
> +#define PROCESS_ARG_POINTER() va_arg (ap, void *)
> +#define PROCESS_ARG_STRING() va_arg (ap, const char *)
> +#define PROCESS_ARG_UNSIGNED_INT() va_arg (ap, unsigned int)
> +#define PROCESS_ARG_UNSIGNED_LONG_INT() va_arg (ap, unsigned long int)
> +#define PROCESS_ARG_UNSIGNED_LONG_LONG_INT() va_arg (ap, unsigned long long int)
> +#define PROCESS_ARG_WCHAR_T() va_arg (ap, wchar_t)
> +#define PROCESS_ARG_WSTRING() va_arg (ap, const wchar_t *)

I think it would be nicer to spell these macros in lower case.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] vfprintf: Unify argument handling in process_arg
  2021-09-22 14:17   ` Andreas Schwab
@ 2021-09-23 10:07     ` Florian Weimer
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Weimer @ 2021-09-23 10:07 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Florian Weimer via Libc-alpha

* Andreas Schwab:

> On Sep 22 2021, Florian Weimer via Libc-alpha wrote:
>
>> Instead of checking a pointer argument for NULL, use helper macros
>> defined differently in the non-positional and positional cases.
>> This avoids frequent conditional checks and a GCC 12 warning
>> about comparing pointers against NULL which cannot be NULL.
>
> Ok.
>
>> @@ -1545,8 +1474,28 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
>>        /* Process current format.  */
>>        while (1)
>>  	{
>> -	  process_arg (((struct printf_spec *) NULL));
>> -	  process_string_arg (((struct printf_spec *) NULL));
>> +#define PROCESS_ARG_INT() va_arg (ap, int)
>> +#define PROCESS_ARG_LONG_INT() va_arg (ap, long int)
>> +#define PROCESS_ARG_LONG_LONG_INT() va_arg (ap, long long int)
>> +#define PROCESS_ARG_POINTER() va_arg (ap, void *)
>> +#define PROCESS_ARG_STRING() va_arg (ap, const char *)
>> +#define PROCESS_ARG_UNSIGNED_INT() va_arg (ap, unsigned int)
>> +#define PROCESS_ARG_UNSIGNED_LONG_INT() va_arg (ap, unsigned long int)
>> +#define PROCESS_ARG_UNSIGNED_LONG_LONG_INT() va_arg (ap, unsigned long long int)
>> +#define PROCESS_ARG_WCHAR_T() va_arg (ap, wchar_t)
>> +#define PROCESS_ARG_WSTRING() va_arg (ap, const wchar_t *)
>
> I think it would be nicer to spell these macros in lower case.

Fair enough, I've renamed the macros to lower case before pushing.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-23 10:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22  7:09 [PATCH 0/2] vfprintf cleanups to avoid -Waddress warning Florian Weimer
2021-09-22  7:10 ` [PATCH 1/2] vfprintf: Handle floating-point cases outside of process_arg macro Florian Weimer
2021-09-22 13:55   ` Andreas Schwab
2021-09-22  7:10 ` [PATCH 2/2] vfprintf: Unify argument handling in process_arg Florian Weimer
2021-09-22 14:17   ` Andreas Schwab
2021-09-23 10:07     ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).