linux-kernel - Re: [PATCH 2/4] pmc551 remove unnecessary braces

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]

Date:	Mon, 18 Sep 2006 19:16:19 -0500
From:	Josh Boyer <jdub@...ibm.com>
To:	Jiri Slaby <jirislaby@...il.com>
Cc:	Andrew Morton <akpm@...l.org>, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Mark Ferrell <mferrell@...sta.com>
Subject: Re: [PATCH 2/4] pmc551 remove unnecessary braces

On Tue, 2006-09-19 at 00:47 +0200, Jiri Slaby wrote:
> 
> diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> index 6d4d5a4..0ee22ca 100644
> --- a/drivers/mtd/devices/pmc551.c
> +++ b/drivers/mtd/devices/pmc551.c
> @@ -137,11 +137,11 @@ #endif
>  
>  	pmc551_point(mtd, instr->addr, instr->len, &retlen, &ptr);
>  
> -	if (soff_hi == eoff_hi || mtd->size == priv->asize) {
> +	if (soff_hi == eoff_hi || mtd->size == priv->asize)
>  		/* The whole thing fits within one access, so just one shot
>  		   will do it. */
>  		memset(ptr, 0xff, instr->len);
> -	} else {
> +	else {
>  		/* We have to do multiple writes to get all the data
>  		   written. */
>  		while (soff_hi != eoff_hi) {

I actually find this change to make the code less readable.  Yes, the
braces aren't technically necessary, but the else requires them, and the
comment block before the memset makes this multi-line.

This whole patch is highly user preference, but I'd rather these braces
stay.


> @@ -700,9 +695,8 @@ static int __init init_pmc551(void)
>  
>  		if ((PCI_Device = pci_find_device(PCI_VENDOR_ID_V3_SEMI,
>  						  PCI_DEVICE_ID_V3_SEMI_V370PDC,
> -						  PCI_Device)) == NULL) {
> +						  PCI_Device)) == NULL)
>  			break;
> -		}

1) If you're going for coding style, the assignment within the if
condition needs to be moved outside of it.

2) If you're not going to fix 1, then leave the braces.

josh

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/