Re: [PATCH] vsprintf: simplify number handling

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

On Tue, Dec 17 2024, Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Instead of dealing with all the different special types (size_t,
> unsigned char, ptrdiff_t..) just deal with the size of the integer type
> and the sign.
>
> This avoids a lot of unnecessary case statements, and the games we play
> with the value of the 'SIGN' flags value
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>
> NOTE! Only very lightly tested.  Also meant to be purely preparatory. 
> It might be broken. 
>
> I started doing this in the hope that the vsnprintf() core code could
> maybe be further cleaned up enough that it would actually be something
> we could use more generally and export to other users that want to
> basically do the printk format handling. 
>
> The code is *not* there yet, though. Small steps.
>

> +/* Turn a 1/2/4-byte value into a 64-bit one with sign handling */
> +static unsigned long long get_num(unsigned int val, struct printf_spec spec)
> +{
> +	unsigned int shift = 32 - spec.type*8;
> +
> +	val <<= shift;
> +	if (!(spec.flags & SIGN))
> +		return val >> shift;
> +	return (int)val >> shift;
> +}
> +

I think this needs to be a little more explicit that it's not just about
handling sign extension, but also truncation to the desired
width. Otherwise somebody will wonder why this isn't

  if (!(spec.flags & SIGN))
    return val;
  return (int)(val << shift) >> shift;

[with the latter line duplicating our sign_extend32 helper].

The rest looks good.

Rasmus