-
-
Notifications
You must be signed in to change notification settings - Fork 71
Description
The C23 standard says:
For d, i, o, u, x, and X conversions, if a precision is specified, the 0 flag is ignored. For other conversions, the behavior is undefined.
But also:
The precision takes the form of a period (.) followed either by an asterisk * (described later) or by an optional nonnegative decimal integer; if only the period is specified, the precision is taken as zero.
A negative precision argument is taken as if the precision were omitted.
This is the output produced by NPF:
printf("%010.*i", -1, 2); // " 2"
and this is by other implementations
printf("%010.*i", -1, 2); // "0000000002"
The bug only appears when a width is specified too, otherwise the '0' flag has no visible effect anyway.
NPF already ignores negative precision values, but in this specific case it ignores it but also ignores the '0' flag (as would be done if the precision was non-negative).
Note that GCC warns about calling printf with such arguments:
'0' flag ignored with precision and '%i' ms_printf format [-Wformat=]
But the warning is bogus: it does not understand that the precision is dynamic, and that it must be ignored if negative (it can't check it in the general case, but in this case it could see that the parameter is the compile-time constant -2).
The problem is that NPF discards the '0' flag immediately as it detects that some precision is specified. It does that before it has extracted the precision argument (in case of ".*"), so it cannot detect that the precision is actually "unspecified" (NPF completely ignores a negative precision, so it behaves --or should behave-- as if no precision was specified at all).
This happens here:
Lines 419 to 426 in a4a26d0
| case 'x': | |
| if (tmp_conv == NPF_FMT_SPEC_CONV_NONE) { tmp_conv = NPF_FMT_SPEC_CONV_HEX_INT; } | |
| out_spec->conv_spec = (uint8_t)tmp_conv; | |
| #if (NANOPRINTF_USE_FIELD_WIDTH_FORMAT_SPECIFIERS == 1) && \ | |
| (NANOPRINTF_USE_PRECISION_FORMAT_SPECIFIERS == 1) | |
| if (out_spec->prec_opt != NPF_FMT_SPEC_OPT_NONE) { out_spec->leading_zero_pad = 0; } | |
| #endif | |
| break; |
in
if (out_spec->prec_opt != NPF_FMT_SPEC_OPT_NONE) { out_spec->leading_zero_pad = 0; }
I'm not sure how you want to go about fixing this. You need to skip this instruction, but you need to reintroduce it later (with modifications) in multiple places -- here you have one single place that acts for all integer specifiers, while later on you have one case for signed and one for unsigned integers.
Also, a similar problems happens with floats:
printf("%010.*f", -5, 1.234); // "001.234000" in other implementations, "0000000err" with NPF
The negative precision causes some overflow in the calculations, and although this is UB too, hence acceptable, it would be nicer to ignore a negative precision. However, a missing float precision must result in the default precision being used (6), which is done around line 415, but this too is skipped if the precision is present (and we don't know yet that we should ignore it).
One solution would be to always fetch the width/precision arguments immediately, instead of waiting for a successful parsing of the conversion specifier. This allows us to know already, here:
Lines 429 to 433 in a4a26d0
| case 'F': out_spec->case_adjust = 0; | |
| case 'f': | |
| out_spec->conv_spec = NPF_FMT_SPEC_CONV_FLOAT_DEC; | |
| if (out_spec->prec_opt == NPF_FMT_SPEC_OPT_NONE) { out_spec->prec = 6; } | |
| break; |
(and following cases) whether the provided precision had to be discarded.
Actually, a negative precision is not UB.
The standard:
A negative precision argument is taken as if the precision were omitted.
So, we must reorganize the parsing so that '0' is not cleared until we know it is ok, ie until after extracting the ".*" argument.
Also, I like the fact that we do not access the arguments until after the specifier is parsed. This avoids more problems in some cases. I would leave this where it is, and postpone after it some of the logic that now takes place inside the parsing function.