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

[Petr Mladek <pmladek@suse.com>]

On Mon 2022-11-14 17:46:28, Andy Shevchenko wrote:
> On Mon, Nov 14, 2022 at 04:34:07PM +0100, Petr Mladek wrote:
> > On Tue 2022-11-08 16:33:22, Russell King wrote:
> 
> ...
> 
> > >  	orig = get_unaligned(fourcc);
> > > -	val = orig & ~BIT(31);
> > > +	switch (fmt[2]) {
> > > +	case 'h':
> > > +		val = orig;
> > > +		break;
> > > +	case 'r':
> > > +		val = orig = swab32(orig);
> > 
> > I do not like much these multi assignments. I think that the result
> > was not even defined in some older C standards. Though, I can't find
> > it now. And even make W=3 does not warn about it.
> > 
> > > +		break;
> > > +	case 'l':
> > > +		val = orig = le32_to_cpu(orig);
> > > +		break;
> > > +	case 'b':
> > > +		val = orig = be32_to_cpu(orig);
> > > +		break;
> 
> Isn't easy to fix? Something like below?
> 
> 	switch (fmt[2]) {
> 	case 'h':
> 		break;
> 	case 'r':
> 		orig = swab32(orig);
> 		break;
> 	case 'l':
> 		orig = le32_to_cpu(orig);
> 		break;
> 	case 'b':
> 		orig = be32_to_cpu(orig);
> 		break;
> 
> 		...
> 	}
> 	val = orig;

I though the same. Unfortunately, this is not valid for the "case c:"
path where "orig" stays untouched:

	case 'c':
		/* Pixel formats are printed LSB-first */
		val = swab32(orig & ~BIT(31));
		pixel_fmt = true;
		break;

It is pity that "orig" is handled differently for the pixel and the generic
formats.

But I am afraid that there is no good solution. The code will
always be a mess when it tries to implement a messy definition.

It would be nice if the the FourCC format was used consistently
in all subsystems in the first place.


IMPORTANT: This brings the questions.

	   Is there actually a standard how to print the original
	   number in FourCC?

	   Do we really want to modify "orig" in the generic
	   implementation?

Best Regards,
Petr