Account for octal marker in %#o format
Checks
Commit Message
---
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
Comments
* 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);
@@ -207,6 +207,7 @@ tests := \
tst-printf-bz25691 \
tst-printf-fp-free \
tst-printf-fp-leak \
+ tst-printf-oct \
tst-printf-round \
tst-printfsz \
tst-put-error \
new file mode 100644
@@ -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>
@@ -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);