linux-kernel - Re: [PATCH] auxdisplay: panel: replace struct with union for display configuration

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: <20241225174648.GA31874@haproxy.com>
Date: Wed, 25 Dec 2024 18:46:48 +0100
From: Willy TARREAU <wtarreau@...roxy.com>
To: Atharva Tiwari <evepolonium@...il.com>
Cc: Ksenija Stanojevic <ksenija.stanojevic@...il.com>,
	Andy Shevchenko <andy@...nel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	"Dr. David Alan Gilbert" <linux@...blig.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] auxdisplay: panel: replace struct with union
 for display configuration

Hello!

On Wed, Dec 25, 2024 at 11:11:18PM +0530, Atharva Tiwari wrote:
> this patch replaces a struct with a union in the panel.c driver 
> to better represent display configuration as mentioned in TODO
> 
> Signed-off-by: Atharva Tiwari <evepolonium@...il.com>
> ---
>  drivers/auxdisplay/panel.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> index a731f28455b4..4662f763dac7 100644
> --- a/drivers/auxdisplay/panel.c
> +++ b/drivers/auxdisplay/panel.c
> @@ -204,8 +204,7 @@ static struct {
>  	int charset;
>  	int proto;
>  
> -	/* TODO: use union here? */
> -	struct {
> +	union {
>  		int e;
>  		int rs;
>  		int rw;

Have you tested this patch ? I guess not. The TODO here is not just to
change a language keyword but to see if it would be better achieved
using a different construct and representation of the different signals.
Here what you've done is merge all the signals into a single one.

I think that a better patch would be to just remove the TODO comment
that has been there for about 20 years without making any progress, and
which is no longer relevant since that code will not change now.

regards,
Willy