[PATCH 4/7] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Petr Mladek]

On Tue 2022-09-06 14:19:44, Russell King wrote:
> From: Hector Martin <marcan at marcan.st>
> 
> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FOURCCs with various endian semantics:
> 
> %p4ch   Host-endian
> %p4cl	Little-endian
> %p4cb	Big-endian
> %p4cr	Reverse-endian
> 
> The endianness determines how bytes are interpreted as a u32, and the
> FOURCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> allow printing LSByte-first FOURCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
> 
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -625,6 +625,38 @@ Passed by reference.
>  	%p4cc	Y10  little-endian (0x20303159)
>  	%p4cc	NV12 big-endian (0xb231564e)
>  
> +Generic FourCC code
> +-------------------
> +
> +::
> +	%p4c[hnbl]	gP00 (0x67503030)
> +
> +Print a generic FourCC code, as both ASCII characters and its numerical
> +value as hexadecimal.
> +
> +The additional ``h``, ``r``, ``b``, and ``l`` specifiers are used to specify
> +host, reversed, big or little endian order data respectively. Host endian
> +order means the data is interpreted as a 32-bit integer and the most
> +significant byte is printed first; that is, the character code as printed
> +matches the byte order stored in memory on big-endian systems, and is reversed
> +on little-endian systems.
> +
> +Passed by reference.
> +
> +Examples for a little-endian machine, given &(u32)0x67503030::
> +
> +	%p4ch	gP00 (0x67503030)
> +	%p4cl	gP00 (0x67503030)
> +	%p4cb	00Pg (0x30305067)
> +	%p4cr	00Pg (0x30305067)

Nit: I would prefer to keep the same order (h,r,b,l) everywhere.

     I guess that you wanted to show exactly the same results next
     to each other. But it is not the case on big-endian anyway.

> +
> +Examples for a big-endian machine, given &(u32)0x67503030::
> +
> +	%p4ch	gP00 (0x67503030)
> +	%p4cl	00Pg (0x30305067)
> +	%p4cb	gP00 (0x67503030)
> +	%p4cr	00Pg (0x30305067)

Same here.

> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3c1853a9d1c0..31707499f90f 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1757,27 +1757,50 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>  	char output[sizeof("0123 little-endian (0x01234567)")];
>  	char *p = output;
>  	unsigned int i;
> +	bool pix_fmt = false;

Nit: I would prefer "pixel_fmt". I am not a graphics guy and wondered
     what "pix" did stands for ;-)

>  	u32 orig, val;
>  
> -	if (fmt[1] != 'c' || fmt[2] != 'c')
> +	if (fmt[1] != 'c')
>  		return error_string(buf, end, "(%p4?)", spec);
>  
>  	if (check_pointer(&buf, end, fourcc, spec))
>  		return buf;
>  
>  	orig = get_unaligned(fourcc);
> -	val = orig & ~BIT(31);
> +	switch (fmt[2]) {
> +	case 'h':
> +		val = orig;
> +		break;
> +	case 'r':
> +		val = orig = swab32(orig);
> +		break;
> +	case 'l':
> +		val = orig = le32_to_cpu(orig);
> +		break;
> +	case 'b':
> +		val = orig = be32_to_cpu(orig);
> +		break;
> +	case 'c':
> +		/* Pixel formats are printed LSB-first */
> +		val = swab32(orig & ~BIT(31));
> +		pix_fmt = true;
> +		break;
> +	default:
> +		return error_string(buf, end, "(%p4?)", spec);
> +	}
>  
>  	for (i = 0; i < sizeof(u32); i++) {
> -		unsigned char c = val >> (i * 8);
> +		unsigned char c = val >> ((3 - i) * 8);

This hardcodes '3' but the for-cycle uses i < sizeof(u32).
We should be consistent.

A solution would be:

	int i;

	for (i = sizeof(u32); --i >= 0;) {
		unsigned char c = val >> (i * 8);


>  		/* Print non-control ASCII characters as-is, dot otherwise */
>  		*p++ = isascii(c) && isprint(c) ? c : '.';
>  	}
>  
> -	*p++ = ' ';
> -	strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> -	p += strlen(p);
> +	if (pix_fmt) {
> +		*p++ = ' ';
> +		strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> +		p += strlen(p);
> +	}
>  
>  	*p++ = ' ';
>  	*p++ = '(';

Best Regards,
Petr