Add POSIX's C conversion specifier to printf funcs #1276
Conversation
| @@ -1073,6 +1073,9 @@ int __fmt(void *fn, void *arg, const char *format, va_list va, int *wrote) { | |||
| } | |||
| break; | |||
| } | |||
| case 'C': | |||
| signbit = 63; | |||
There was a problem hiding this comment.
wchar_t / wint_t are 32-bit types. Why is signbit set to 63?
There was a problem hiding this comment.
__fmt_stoa only considers the argument to be of type wchar_t if signbit == 63. Otherwise, it's char16_t if signbit == 15 and otherwise it's char. I suppose the way wchar_t conversions in general could be changed so it uses a different value of signbit, but without such major changes to other parts of fmt.c, there isn't really any way to implement the C conversion specifier without having signbit end up with a value of 63
There was a problem hiding this comment.
Will it say va_arg(va, long)? Because if that's the case, you should write a test that does printf("%d%d%d%d%d%d%d%d%C%d\n", 0,0,0,0,0,0,0,0, L'x', 1) to prove it won't break.
There was a problem hiding this comment.
If that breaks %C, then I would also expect it to break %lc (I'll try it, though)
There was a problem hiding this comment.
Neither break, I'll amend my commit with the corresponding test.
There was a problem hiding this comment.
(this is because the code in case 'c': 2 lines later unconditionally does va_arg(va, int) - for this purpose, it does not care about signbit)
There was a problem hiding this comment.
I've pushed the amended commit.
There was a problem hiding this comment.
(uh, now it should actually work, sorry for that lol)
GabrielRavier
force-pushed
the
fix/printf-C-conversion-specifier
branch
2 times, most recently
from
2aa8402 to
651c081
Compare
POSIX specifies the C conversion specifier as being "equivalent to %lc",
i.e. printf("%C", arg) is equivalent in behaviour to printf("%lc", arg).
This patch implements this conversion specifier, and adds a test for it,
alongside another test, which ensures that va_arg uses the correct size,
even though we set signbit to 63 in the code (which one might think will
result in the wrong size of argument being va_arg-ed, but having signbit
set to 63 is in fact what __fmt_stoa expects and is a requirement for it
properly formatting the wchar_t argument - this does not result in wrong
usage of va_arg because the implementation of the c conversion specifier
(which the implementation of the C conversion specifier fallsthrough to)
always calls va_arg with an argument type of int, to avoid the very same
bug occuring with %lc, as the l length modifier also sets signbit to 63)
GabrielRavier
force-pushed
the
fix/printf-C-conversion-specifier
branch
from
651c081 to
23d3c12
Compare
POSIX specifies the C conversion specifier as being "equivalent to %lc",
i.e. printf("%C", arg) is equivalent in behaviour to printf("%lc", arg).
This patch implements this conversion specifier, and adds a test for it,
alongside another test, which ensures that va_arg uses the correct size,
even though we set signbit to 63 in the code (which one might think will
result in the wrong size of argument being va_arg-ed, but having signbit
set to 63 is in fact what __fmt_stoa expects and is a requirement for it
properly formatting the wchar_t argument - this does not result in wrong
usage of va_arg because the implementation of the c conversion specifier
(which the implementation of the C conversion specifier fallsthrough to)
always calls va_arg with an argument type of int, to avoid the very same
bug occuring with %lc, as the l length modifier also sets signbit to 63)