Re: [PATCH] Account for grouping in printf width (bug 23432)

[Carlos O'Donell <carlos@redhat.com>]

On 1/19/23 06:50, Andreas Schwab via Libc-alpha wrote:
> This is a partial fix for mishandling of grouping when formatting
> integers.  It properly computes the width in presence of grouping
> characteres when the precision is larger than the number of significant
> digits.

I have a new version of this patch which I'll repost shortly after my regression testing
passes.

I would like to make a slight change in the existing code because prec is changed to
mean something else completely in the middle of the digit processing. We should introduce
a new usigned integer value to carry forward the additional bytes needed for precision
and let the compiler optimize that. It is IMO a small enough change that it still fits
into the category of making the code cleaner and solving the regression. I say this because
we touch a line that makes no sense to me since 'prec' since the start of processing
has meant the total precision parsed from the format specifier.

I can also see where we go awry in bug 23432, but I'm really nervous to touch that because
existing code may expect the existing practice. Changing to match POSIX is going to be
an interesting threading of the needle, either developers never care about this use case
or POSIX is just being weird in requiring the additional leading zeroes to have grouping
characters.

> ---
>  stdio-common/Makefile               |  1 +
>  stdio-common/tst-grouping3.c        | 37 +++++++++++++++++++++++++++++
>  stdio-common/vfprintf-process-arg.c |  2 +-
>  3 files changed, 39 insertions(+), 1 deletion(-)
>  create mode 100644 stdio-common/tst-grouping3.c
> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 6e9d104524..b46d932a20 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -195,6 +195,7 @@ tests := \
>    tst-gets \
>    tst-grouping \
>    tst-grouping2 \
> +  tst-grouping3 \
>    tst-long-dbl-fphex \
>    tst-memstream-string \
>    tst-obprintf \
> diff --git a/stdio-common/tst-grouping3.c b/stdio-common/tst-grouping3.c
> new file mode 100644
> index 0000000000..0031ad4010
> --- /dev/null
> +++ b/stdio-common/tst-grouping3.c
> @@ -0,0 +1,37 @@
> +/* Test printf with grouping and padding (bug 23432)
> +   Copyright (C) 2023 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 <locale.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +static int
> +do_test (void)
> +{
> +  char buf[80];
> +
> +  xsetlocale (LC_NUMERIC, "de_DE.UTF-8");
> +
> +  sprintf (buf, "%+-'13.9d", 1234567);
> +  TEST_COMPARE_STRING (buf, "+001.234.567 ");
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/stdio-common/vfprintf-process-arg.c b/stdio-common/vfprintf-process-arg.c
> index 2c651946df..cd3eaf5c0c 100644
> --- a/stdio-common/vfprintf-process-arg.c
> +++ b/stdio-common/vfprintf-process-arg.c
> @@ -257,7 +257,7 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
>            width -= 2;
>          }
>  
> -      width -= workend - string + prec;
> +      width -= number_length + prec;
>  
>        Xprintf_buffer_pad (buf, L_('0'), prec);
>  

-- 
Cheers,
Carlos.