Account for octal marker in %#o format

  ---
 stdio-common/Makefile               |  1 +
 stdio-common/tst-printf-oct.c       | 50 +++++++++++++++++++++++++++++
 stdio-common/vfprintf-process-arg.c |  6 ++++
 3 files changed, 57 insertions(+)
 create mode 100644 stdio-common/tst-printf-oct.c
  

* Andreas Schwab via Libc-alpha:

> @@ -257,6 +260,9 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
>            width -= 2;
>          }
>  
> +      if (octal_marker)
> +	--width;
> +
>        width -= number_length + prec;

This doesn't apply because master still has

      width -= workend - string + prec;

Apart from that, the change looks okay.

Carlos, this would be a regression, so I think we should fix it before
the release.

Thanks,
Florian
  

On 1/30/23 06:11, Florian Weimer wrote:
> * Andreas Schwab via Libc-alpha:
> 
>> @@ -257,6 +260,9 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
>>            width -= 2;
>>          }
>>  
>> +      if (octal_marker)
>> +	--width;
>> +
>>        width -= number_length + prec;
> 
> This doesn't apply because master still has
> 
>       width -= workend - string + prec;
> 
> Apart from that, the change looks okay.
> 
> Carlos, this would be a regression, so I think we should fix it before
> the release.

Agreed. We should fix this ASAP for the 2.37 release.

Andreas, Thanks for putting together a fix. I'm reviewing this now.
  

On 1/30/23 04:53, Andreas Schwab via Libc-alpha wrote:

We don't need a bug number because this issues was never visible
to a user in a release.

I applied the fix carefully by hand, since as Florian notes this
seems to be from a branch where you are working on your other
fixes that are under review.

Confirmed test fails without the change on x86_64 and i686 and
passes after the fix. It is not impacted by your other changes
as far as I can tell. It correctly fixes the required space
calculation in both branches of the argument processing, and that
looks like the correct fix to me.

Note that CI/CD failed because patch fails to apply, but we
still want this fix in ASAP before the release.

OK for 2.37. Please commit ASAP.

No regressions on x86-64 and i686.

Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  stdio-common/Makefile               |  1 +
>  stdio-common/tst-printf-oct.c       | 50 +++++++++++++++++++++++++++++
>  stdio-common/vfprintf-process-arg.c |  6 ++++
>  3 files changed, 57 insertions(+)
>  create mode 100644 stdio-common/tst-printf-oct.c
> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 8150453944..c787450e5c 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -207,6 +207,7 @@ tests := \
>    tst-printf-bz25691 \
>    tst-printf-fp-free \
>    tst-printf-fp-leak \
> +  tst-printf-oct \

OK. Sorted correctly. Adds new test for regression.

>    tst-printf-round \
>    tst-printfsz \
>    tst-put-error \
> diff --git a/stdio-common/tst-printf-oct.c b/stdio-common/tst-printf-oct.c
> new file mode 100644
> index 0000000000..d1c88d5641
> --- /dev/null
> +++ b/stdio-common/tst-printf-oct.c
> @@ -0,0 +1,50 @@
> +/* Test printf width padding with octal format
> +   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];
> +
> +  sprintf (buf, "%-7o", 123);
> +  TEST_COMPARE_STRING (buf, "173    ");
> +
> +  sprintf (buf, "%#-7o", 123);
> +  TEST_COMPARE_STRING (buf, "0173   ");
> +
> +  sprintf (buf, "%7o", 123);
> +  TEST_COMPARE_STRING (buf, "    173");
> +
> +  sprintf (buf, "%#7o", 123);
> +  TEST_COMPARE_STRING (buf, "   0173");
> +
> +  sprintf (buf, "%07o", 123);
> +  TEST_COMPARE_STRING (buf, "0000173");
> +
> +  sprintf (buf, "%#07o", 123);
> +  TEST_COMPARE_STRING (buf, "0000173");
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/stdio-common/vfprintf-process-arg.c b/stdio-common/vfprintf-process-arg.c
> index cd3eaf5c0c..279a0efa9d 100644
> --- a/stdio-common/vfprintf-process-arg.c
> +++ b/stdio-common/vfprintf-process-arg.c
> @@ -196,6 +196,9 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
>          /* Account for 0X, 0x, 0B or 0b hex or binary marker.  */
>          width -= 2;
>  
> +      if (octal_marker)
> +        --width;
> +
>        if (is_negative || showsign || space)
>          --width;
>  
> @@ -257,6 +260,9 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
>            width -= 2;
>          }
>  
> +      if (octal_marker)
> +	--width;
> +
>        width -= number_length + prec;
>  
>        Xprintf_buffer_pad (buf, L_('0'), prec);