Re: [PATCH v3 3/3] vsprintf: Move space out of string literals in fourcc_string()

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Kees,

On Wed, Jan 26, 2022 at 01:22:32PM -0800, Kees Cook wrote:
> On Wed, Jan 26, 2022 at 04:19:17PM +0200, Andy Shevchenko wrote:
> > The literals "big-endian" and "little-endian" may be potentially
> > occurred in other places. Dropping space allows linker to
> > "compress" them by using only a single copy.
> > 
> > Rasmus suggested, while at it, replacing strcpy() + strlen() by
> > p = stpcpy(), which is done here as well.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > v3: no changes
> >  lib/vsprintf.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 4e8f3e9acb99..e2a1d89f1a5c 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1781,8 +1781,8 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> >  		*p++ = isascii(c) && isprint(c) ? c : '.';
> >  	}
> >  
> > -	strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian");
> > -	p += strlen(p);
> > +	*p++ = ' ';
> > +	p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> >  
> >  	*p++ = ' ';
> >  	*p++ = '(';
> 
> No need for any of the manual construction nor stpcpy():
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index d7ad44f2c8f5..aef8bd2789a9 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1788,14 +1788,9 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>  		*p++ = isascii(c) && isprint(c) ? c : '.';
>  	}
>  
> -	strcpy(p, *fourcc & BIT(31) ? " big-endian" : " little-endian");
> -	p += strlen(p);
> -
> -	*p++ = ' ';
> -	*p++ = '(';
> -	p = special_hex_number(p, output + sizeof(output) - 2, *fourcc, sizeof(u32));
> -	*p++ = ')';
> -	*p = '\0';
> +	snprintf(p, sizeof(output) - sizeof(*fourcc), " %s (0x%x)",
> +		*fourcc & BIT(31) ? "big-endian" : " little-endian",
> +		*fourcc);
>  
>  	return string(buf, end, output, spec);
>  }
> 
> 
> (untested)

This could work but results in two nested calls to vsnprintf(). I'd
definitely avoid that, due to stack usage and then albeit it's easy to see
the recursion will end with two iterations, that will very fast cease to be
the case if you keep applying the pattern.

-- 
Regards,

Sakari Ailus