Subject: Re: bug#73068: printf: please implement POSIX:2024 argument reordering

From: Pádraig Brady <[email-masked]>
Date: Thu, 12 Sep 2024 20:03:51 +0100

On 12/09/2024 18:40, Collin Funk wrote:
> Hi Pádraig,
> 
> Pádraig Brady <[email-masked]> writes:
> 
>> I'll apply the attached sometime tomorrow.
>>
>> Marking this as done.
> 
> Patch looks good, thanks.
> 
> One small comment, though.
> 
>> +#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 == '$')	\
>> +    /* Process indexed %i$ format.  */			\
>> +    /* Note '$' comes before any flags.  */		\
> 
> Shouldn't you check errno here, like:
> 
>    char *arge;
>    errno = 0;
>    intmax_t arg = POS==3 ? 0 : strtoimax (f, &arge, 10);
>    if (errno == 0 && 0 < arg && arg <= INT_MAX && *arge == '$')
>    [...]
> 
> I think that would handle all bad cases.
> 
> For example, I think "%$" might return 0 but set errno to EINVAL.

A fair point, but note we only accept 1 ... INT_MAX,
so that implicitly excludes any of the possible error returns.
I should at least add a comment.

Though it got me thinking that strtol() may be too lenient
in what it accepts, resulting in possible confusion for users.
For example some printf flags like ' ' or '+' would be accepted
as part of a number, when ideally they should not be.

For example, the user might do:

$ printf '[% 1$d]\n' 1234
[1234]

When they really intended:

$ printf '[%1$ d]\n' 1234
[ 1234]

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.

thanks!
Pádraig