vfprintf: Reuse work_buffer in group_number

Message ID 20170619162002.68BAE402AEC0E@oldenburg.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer June 19, 2017, 4:20 p.m. UTC
  2017-06-19  Florian Weimer  <fweimer@redhat.com>

	* stdio-common/vfprintf.c (group_number): Add front_ptr argument.
	Use it to make the temporary copy at the start of the work buffer.
	(process_arg): Adjust call to group_number.
  

Comments

Adhemerval Zanella June 27, 2017, 6:07 p.m. UTC | #1
On 19/06/2017 13:20, Florian Weimer wrote:
> 2017-06-19  Florian Weimer  <fweimer@redhat.com>
> 
> 	* stdio-common/vfprintf.c (group_number): Add front_ptr argument.
> 	Use it to make the temporary copy at the start of the work buffer.
> 	(process_arg): Adjust call to group_number.

LGTM, thanks.


> 
> diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
> index f0ea4fe..e0c6edf 100644
> --- a/stdio-common/vfprintf.c
> +++ b/stdio-common/vfprintf.c
> @@ -594,9 +594,8 @@ static const uint8_t jump_table[] =
>  	      string = _itoa (number.longlong, workend, base,		      \
>  			      spec == L_('X'));				      \
>  	      if (group && grouping)					      \
> -		string = group_number (string, workend, grouping,	      \
> -				       thousands_sep);			      \
> -									      \
> +		string = group_number (work_buffer, string, workend,	      \
> +				       grouping, thousands_sep);	      \
>  	      if (use_outdigits && base == 10)				      \
>  		string = _i18n_number_rewrite (string, workend, workend);     \
>  	    }								      \
> @@ -652,9 +651,8 @@ static const uint8_t jump_table[] =
>  	      string = _itoa_word (number.word, workend, base,		      \
>  				   spec == L_('X'));			      \
>  	      if (group && grouping)					      \
> -		string = group_number (string, workend, grouping,	      \
> -				       thousands_sep);			      \
> -									      \
> +		string = group_number (work_buffer, string, workend,	      \
> +				       grouping, thousands_sep);	      \
>  	      if (use_outdigits && base == 10)				      \
>  		string = _i18n_number_rewrite (string, workend, workend);     \
>  	    }								      \
> @@ -1236,8 +1234,8 @@ static int printf_unknown (FILE *, const struct printf_info *,
>  			   const void *const *) __THROW;
>  
>  /* Group digits of number string.  */
> -static CHAR_T *group_number (CHAR_T *, CHAR_T *, const char *, THOUSANDS_SEP_T)
> -     __THROW internal_function;
> +static CHAR_T *group_number (CHAR_T *, CHAR_T *, CHAR_T *, const char *,
> +			     THOUSANDS_SEP_T);
>  
>  /* The function itself.  */
>  int
> @@ -2012,16 +2010,20 @@ printf_unknown (FILE *s, const struct printf_info *info,
>    return done;
>  }
>  
> -/* Group the digits according to the grouping rules of the current locale.
> -   The interpretation of GROUPING is as in `struct lconv' from <locale.h>.  */
> +/* Group the digits from W to REAR_PTR according to the grouping rules
> +   of the current locale.  The interpretation of GROUPING is as in
> +   `struct lconv' from <locale.h>.  The grouped number extends from
> +   the returned pointer until REAR_PTR.  FRONT_PTR to W is used as a
> +   scratch area.  */
>  static CHAR_T *
> -internal_function
> -group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
> -	      THOUSANDS_SEP_T thousands_sep)
> +group_number (CHAR_T *front_ptr, CHAR_T *w, CHAR_T *rear_ptr,
> +	      const char *grouping, THOUSANDS_SEP_T thousands_sep)
>  {
> +  /* Length of the current group.  */
>    int len;
> -  CHAR_T *src, *s;
>  #ifndef COMPILE_WPRINTF
> +  /* Length of the separator (in wide mode, the separator is always a
> +     single wide character).  */
>    int tlen = strlen (thousands_sep);
>  #endif
>  
> @@ -2034,26 +2036,34 @@ group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
>    len = *grouping++;
>  
>    /* Copy existing string so that nothing gets overwritten.  */
> -  src = (CHAR_T *) alloca ((rear_ptr - w) * sizeof (CHAR_T));
> -  s = (CHAR_T *) __mempcpy (src, w,
> -			    (rear_ptr - w) * sizeof (CHAR_T));
> +  memmove (front_ptr, w, (rear_ptr - w) * sizeof (CHAR_T));
> +  CHAR_T *s = front_ptr + (rear_ptr - w);
> +
>    w = rear_ptr;
>  
>    /* Process all characters in the string.  */
> -  while (s > src)
> +  while (s > front_ptr)
>      {
>        *--w = *--s;
>  
> -      if (--len == 0 && s > src)
> +      if (--len == 0 && s > front_ptr)
>  	{
>  	  /* A new group begins.  */
>  #ifdef COMPILE_WPRINTF
> -	  *--w = thousands_sep;
> +	  if (w != s)
> +	    *--w = thousands_sep;
> +	  else
> +	    /* Not enough room for the separator.  */
> +	    goto copy_rest;
>  #else
>  	  int cnt = tlen;
> -	  do
> -	    *--w = thousands_sep[--cnt];
> -	  while (cnt > 0);
> +	  if (tlen < w - s)
> +	    do
> +	      *--w = thousands_sep[--cnt];
> +	    while (cnt > 0);
> +	  else
> +	    /* Not enough room for the separator.  */
> +	    goto copy_rest;
>  #endif
>  
>  	  if (*grouping == CHAR_MAX
> @@ -2062,17 +2072,16 @@ group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
>  #endif
>  		   )
>  	    {
> -	      /* No further grouping to be done.
> -		 Copy the rest of the number.  */
> -	      do
> -		*--w = *--s;
> -	      while (s > src);
> +	    copy_rest:
> +	      /* No further grouping to be done.  Copy the rest of the
> +		 number.  */
> +	      memmove (w, s, (front_ptr -s) * sizeof (CHAR_T));
>  	      break;
>  	    }
>  	  else if (*grouping != '\0')
> -	    /* The previous grouping repeats ad infinitum.  */
>  	    len = *grouping++;
>  	  else
> +	    /* The previous grouping repeats ad infinitum.  */
>  	    len = grouping[-1];
>  	}
>      }
>
  

Patch

diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index f0ea4fe..e0c6edf 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -594,9 +594,8 @@  static const uint8_t jump_table[] =
 	      string = _itoa (number.longlong, workend, base,		      \
 			      spec == L_('X'));				      \
 	      if (group && grouping)					      \
-		string = group_number (string, workend, grouping,	      \
-				       thousands_sep);			      \
-									      \
+		string = group_number (work_buffer, string, workend,	      \
+				       grouping, thousands_sep);	      \
 	      if (use_outdigits && base == 10)				      \
 		string = _i18n_number_rewrite (string, workend, workend);     \
 	    }								      \
@@ -652,9 +651,8 @@  static const uint8_t jump_table[] =
 	      string = _itoa_word (number.word, workend, base,		      \
 				   spec == L_('X'));			      \
 	      if (group && grouping)					      \
-		string = group_number (string, workend, grouping,	      \
-				       thousands_sep);			      \
-									      \
+		string = group_number (work_buffer, string, workend,	      \
+				       grouping, thousands_sep);	      \
 	      if (use_outdigits && base == 10)				      \
 		string = _i18n_number_rewrite (string, workend, workend);     \
 	    }								      \
@@ -1236,8 +1234,8 @@  static int printf_unknown (FILE *, const struct printf_info *,
 			   const void *const *) __THROW;
 
 /* Group digits of number string.  */
-static CHAR_T *group_number (CHAR_T *, CHAR_T *, const char *, THOUSANDS_SEP_T)
-     __THROW internal_function;
+static CHAR_T *group_number (CHAR_T *, CHAR_T *, CHAR_T *, const char *,
+			     THOUSANDS_SEP_T);
 
 /* The function itself.  */
 int
@@ -2012,16 +2010,20 @@  printf_unknown (FILE *s, const struct printf_info *info,
   return done;
 }
 
-/* Group the digits according to the grouping rules of the current locale.
-   The interpretation of GROUPING is as in `struct lconv' from <locale.h>.  */
+/* Group the digits from W to REAR_PTR according to the grouping rules
+   of the current locale.  The interpretation of GROUPING is as in
+   `struct lconv' from <locale.h>.  The grouped number extends from
+   the returned pointer until REAR_PTR.  FRONT_PTR to W is used as a
+   scratch area.  */
 static CHAR_T *
-internal_function
-group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
-	      THOUSANDS_SEP_T thousands_sep)
+group_number (CHAR_T *front_ptr, CHAR_T *w, CHAR_T *rear_ptr,
+	      const char *grouping, THOUSANDS_SEP_T thousands_sep)
 {
+  /* Length of the current group.  */
   int len;
-  CHAR_T *src, *s;
 #ifndef COMPILE_WPRINTF
+  /* Length of the separator (in wide mode, the separator is always a
+     single wide character).  */
   int tlen = strlen (thousands_sep);
 #endif
 
@@ -2034,26 +2036,34 @@  group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
   len = *grouping++;
 
   /* Copy existing string so that nothing gets overwritten.  */
-  src = (CHAR_T *) alloca ((rear_ptr - w) * sizeof (CHAR_T));
-  s = (CHAR_T *) __mempcpy (src, w,
-			    (rear_ptr - w) * sizeof (CHAR_T));
+  memmove (front_ptr, w, (rear_ptr - w) * sizeof (CHAR_T));
+  CHAR_T *s = front_ptr + (rear_ptr - w);
+
   w = rear_ptr;
 
   /* Process all characters in the string.  */
-  while (s > src)
+  while (s > front_ptr)
     {
       *--w = *--s;
 
-      if (--len == 0 && s > src)
+      if (--len == 0 && s > front_ptr)
 	{
 	  /* A new group begins.  */
 #ifdef COMPILE_WPRINTF
-	  *--w = thousands_sep;
+	  if (w != s)
+	    *--w = thousands_sep;
+	  else
+	    /* Not enough room for the separator.  */
+	    goto copy_rest;
 #else
 	  int cnt = tlen;
-	  do
-	    *--w = thousands_sep[--cnt];
-	  while (cnt > 0);
+	  if (tlen < w - s)
+	    do
+	      *--w = thousands_sep[--cnt];
+	    while (cnt > 0);
+	  else
+	    /* Not enough room for the separator.  */
+	    goto copy_rest;
 #endif
 
 	  if (*grouping == CHAR_MAX
@@ -2062,17 +2072,16 @@  group_number (CHAR_T *w, CHAR_T *rear_ptr, const char *grouping,
 #endif
 		   )
 	    {
-	      /* No further grouping to be done.
-		 Copy the rest of the number.  */
-	      do
-		*--w = *--s;
-	      while (s > src);
+	    copy_rest:
+	      /* No further grouping to be done.  Copy the rest of the
+		 number.  */
+	      memmove (w, s, (front_ptr -s) * sizeof (CHAR_T));
 	      break;
 	    }
 	  else if (*grouping != '\0')
-	    /* The previous grouping repeats ad infinitum.  */
 	    len = *grouping++;
 	  else
+	    /* The previous grouping repeats ad infinitum.  */
 	    len = grouping[-1];
 	}
     }