Re: [musl] [PATCH v2 1/1] vfprintf: support C2x %b and %B conversion specifiers

[Rich Felker <dalias@libc.org>]

On Sun, Apr 16, 2023 at 03:20:59PM +0200, Gabriel Ravier wrote:
> On 4/16/23 08:51, Jₑₙₛ Gustedt wrote:
> >Gabriel,
> >it also seems to me that ...
> >
> >on Sat, 15 Apr 2023 14:28:28 +0200 you (Gabriel Ravier
> ><gabravier@gmail.com>) wrote:
> >
> >>+	/* This buffer is used for integer conversions. As such, it needs
> >>+	 * to be able to contain the full representation of a number in base 2,
> >>+	 * 8, 10 or 16, with base 2 having the largest possible requirement of
> >>+	 * as many characters as the amount of bits in the largest
> >>possible
> >>+	 * integer type */
> >>+	char buf[sizeof(uintmax_t)*CHAR_BIT];
> >... here a `+3` seems to be in order to take care of the `0[bx]`
> >prefix and a terminating null byte.
> 
> This buffer is only used specifically for storing converted digits,
> and is never used to store the alternative form, and never contains
> a null terminator either as the code knows the used length and never
> passes the buffer to a function that doesn't do so, so from what I
> can see these objections are wrong (in fact it wouldn't make much
> sense to store the prefix in that buffer given that the code also
> has to handle the possibility of extremely large 0-padding that goes
> between the prefix and the converted digits).
> 
> Though perhaps the comment could be improved, I suppose it could be
> confusing...

Yes, without looking I'm pretty sure you're right about what the
buffer is and isn't used for. I think both the +3 and the
+LDBL_MANT_DIG/4 are artifacts of proto-musl code long ago where
things were done differently and floating point code was also in the
main printf_core function and did naive floating point math. Maybe
these should be fixed with a separate patch first (which I could make
and explain what I recall of the history) so this unrelated change
isn't a distraction from the %b patches.

Rich