linux-kernel - Re: [PATCH v4] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8GpZo3M_QSJo2ka@smile.fi.intel.com>
Date: Fri, 28 Feb 2025 14:17:42 +0200
From: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>
To: Aditya Garg <gargaditya08@...e.com>
Cc: "pmladek@...e.com" <pmladek@...e.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	"senozhatsky@...omium.org" <senozhatsky@...omium.org>,
	"corbet@....net" <corbet@....net>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"apw@...onical.com" <apw@...onical.com>,
	"joe@...ches.com" <joe@...ches.com>,
	"dwaipayanray1@...il.com" <dwaipayanray1@...il.com>,
	"lukas.bulwahn@...il.com" <lukas.bulwahn@...il.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Hector Martin <marcan@...can.st>,
	"sven@...npeter.dev" <sven@...npeter.dev>,
	Janne Grunau <j@...nau.net>,
	"alyssa@...enzweig.io" <alyssa@...enzweig.io>,
	"asahi@...ts.linux.dev" <asahi@...ts.linux.dev>
Subject: Re: [PATCH v4] lib/vsprintf: Add support for generic FOURCCs by
 extending %p4cc

On Thu, Feb 27, 2025 at 06:58:24PM +0000, Aditya Garg wrote:
> > On 27 Feb 2025, at 10:40 PM, Aditya Garg <gargaditya08@...e.com> wrote:
> >>> On 27 Feb 2025, at 8:13 PM, andriy.shevchenko@...ux.intel.com wrote:
> >>> On Thu, Feb 27, 2025 at 06:30:48AM +0000, Aditya Garg wrote:

...

> >>> +    switch (fmt[2]) {
> >>> +    case 'h':
> >>> +        val = orig;
> >>> +        break;
> >>> +    case 'r':
> >>> +        orig = swab32(orig);
> >>> +        val = orig;
> >>> +        break;
> >>> +    case 'l':
> >>> +        orig = (__force u32)cpu_to_le32(orig);
> >>> +        val = orig;
> >>> +        break;
> >>> +    case 'b':
> >>> +        orig = (__force u32)cpu_to_be32(orig);
> >>> +        val = orig;
> >>> +        break;
> >>> +    case 'c':
> >>> +        /* Pixel formats are printed LSB-first */
> >>> +        val = swab32(orig & ~BIT(31));
> >>> +        pixel_fmt = true;
> >>> +        break;
> >>> +    default:
> >>> +        return error_string(buf, end, "(%p4?)", spec);
> >>> +    }
> >> 
> >> Actually you can replace all these orig copies by introducing a new boolean, pixel_be.
> >> 
> >> Will become
> >> 
> >>   switch (fmt[2]) {
> >>   case 'h':
> >>       val = orig;
> >>       break;
> >>   case 'r': // or 'n' ?
> >>       val = swab32(orig);
> >>       break;
> >>   case 'l':
> >>       val = (__force u32)cpu_to_le32(orig);
> >>       break;
> >>   case 'b':
> >>       val = (__force u32)cpu_to_be32(orig);
> >>       break;
> >>   case 'c':
> >>       pixel_fmt = true;
> >>       pixel_be = orig & BIT(31);
> >>       /* Pixel formats are printed LSB-first */
> >>       val = swab32(orig & ~BIT(31));
> >>       break;
> >>   default:
> >>       return error_string(buf, end, "(%p4?)", spec);
> >>   }
> >> 
> >> And with this the existence of 'val' now becomes doubtful, we may reuse 'orig',
> >> just name it 'val' everywhere, no?
> > 
> > In case c, val != orig, in rest it is. We can just use pixel_fmt to check this condition, but places where we use orig, and not val will need an if statement or something similar. Tbh, it's an unecessary complication. You might want to see this part of the code:
> 
> More easier IMO can be:
> 
> val = pixel_fmt ? swab32(orig & ~BIT(31)) : orig;
> 
> At the end of the table

Also will work for me. Just think about it, i.e. how to make code shorter and
still readable.

-- 
With Best Regards,
Andy Shevchenko