On 12/09/2024 20:33, Paul Eggert wrote:
On 2024-09-12 12:03, Pádraig Brady wrote:
This is tricky enough, that we should be as restrictive as possible here,
so I may resort to strspn(f, "0123456789") to parse instead.
I'll think a bit about it.
The code's also assuming INT_MAX < INTMAX_MAX, which POSIX doesn't
require. You could put in a static_assert to that effect, I suppose, to
document the assumption.
Indeed. We would have incorrectly taken the INTMAX_MAX arg in the
(albeit unlikely) case where INT_MAX >= INTMAX_MAX,
and the provided number overflowed INTMAX_MAX.
To be explicit, strtol() doesn't return 0 in that case,
so we need to check overflow (like Colin suggested).
More important, though, if you're not in the C locale all bets are off
as far as what strtoimax will also parse.
When I ran into this problem with GNU tar, I ended by giving up on
strtoimax and did my own little integer parser. It does exactly what I
want and I don't have to fire up the strtoimax complexity+locale engine.
Right, it's best to preparse for the above reason,
and to avoid any confusion re leading spaces etc. like I previously mentioned.
The attached adjustment does the preparse with strspn(),
and only does the strtoimax() for appropriate strings.
cheers,
Pádraig
diff --git a/src/printf.c b/src/printf.c
index 37844f14a..442935705 100644
--- a/src/printf.c
+++ b/src/printf.c
@@ -454,14 +454,23 @@ print_formatted (char const *format, int argc, char **argv)
#define GET_CURR_ARG(POS) \
do { \
- char *arge; \
- intmax_t arg = POS==3 ? 0 : strtoimax (f, &arge, 10); \
- if (0 < arg && arg <= INT_MAX && *arge == '$') \
+ intmax_t arg = 0; \
+ size_t argl; \
+ /* Check with strspn() first to avoid spaces etc. \
+ This also avoids any locale ambiguities. */ \
+ if (POS != 3 && (argl = strspn (f, "0123456789")) \
+ && f[argl] == '$') \
+ { \
+ errno = 0; \
+ arg = strtoimax (f, NULL, 10); \
+ if (errno) \
+ arg = 0; \
+ } \
+ if (1 <= arg && arg <= INT_MAX) \
/* Process indexed %i$ format. */ \
- /* Note '$' comes before any flags. */ \
{ \
SET_CURR_ARG (arg - 1); \
- f = arge + 1; \
+ f += argl + 1; \
if (POS == 0) \
direc_arg = arg - 1; \
} \
diff --git a/tests/printf/printf-indexed.sh b/tests/printf/printf-indexed.sh
index d0f799ab1..5d63d68f8 100755
--- a/tests/printf/printf-indexed.sh
+++ b/tests/printf/printf-indexed.sh
@@ -75,19 +75,28 @@ printf_check ' 1' '%2$*1$d\n' 4 1
# Indexed arg, and sequential width
printf_check ' 1' '%2$*d\n' 4 1
-# Flags come after $ (0 is not a flag here):
+# Flags come after $ (0 is not a flag here but allowed):
printf_check ' 1' '%01$4d\n' 1
# Flags come after $ (0 is a flag here):
printf_check '0001' '%1$0*2$d\n' 1 4
# Flags come after $ (-2 not taken as a valid index here):
printf_check_err 'printf: %-2$: invalid conversion specification' \
'%-2$s %1$s\n' A B
+# Flags come after $ (' ' is not allowed as part of number here)
+printf_check_err 'printf: % 2$: invalid conversion specification' \
+ '% 2$s %1$s\n' A B
# Ensure only base 10 numbers are accepted
printf_check_err "printf: 'A': expected a numeric value" \
'%0x2$s %2$s\n' A B
+# Ensure empty numbers are rejected
+printf_check_err 'printf: %$: invalid conversion specification' \
+ '%$d\n' 1
# Verify int limits (avoiding comparisons with argc etc.)
printf_check_err "printf: %${INT_OFLOW}\$: invalid conversion specification" \
"%${INT_OFLOW}\$d\n" 1
+# Verify intmax limits, ensuring overflow detected
+printf_check_err "printf: %${INTMAX_OFLOW}\$: invalid conversion specification" \
+ "%${INTMAX_OFLOW}\$d\n" 1
Exit $fail
