On 12/09/2024 18:40, Collin Funk wrote:
Hi Pádraig,
Pádraig Brady <p...@draigbrady.com> 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
