Patchwork net/pasemi: Fix missing coding style

login
register
mail settings
Submitter Syam Sidhardhan
Date Feb. 24, 2013, 11:01 p.m.
Message ID <1361746879-29261-1-git-send-email-s.syam@samsung.com>
Download mbox | patch
Permalink /patch/222803/
State Accepted
Delegated to: David Miller
Headers show

Comments

Syam Sidhardhan - Feb. 24, 2013, 11:01 p.m.
Fix missing () & { }

Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
 drivers/net/ethernet/pasemi/pasemi_mac.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Joe Perches - Feb. 24, 2013, 11:23 p.m.
On Mon, 2013-02-25 at 04:31 +0530, Syam Sidhardhan wrote:
> Fix missing () & { }
[]
> diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
[]
> @@ -1829,10 +1831,11 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		dev_err(&mac->pdev->dev, "register_netdev failed with error %d\n",
>  			err);
>  		goto out;
> -	} else if netif_msg_probe(mac)
> +	} else if (netif_msg_probe(mac)) {
>  		printk(KERN_INFO "%s: PA Semi %s: intf %d, hw addr %pM\n",
>  		       dev->name, mac->type == MAC_TYPE_GMAC ? "GMAC" : "XAUI",
>  		       mac->dma_if, dev->dev_addr);
> +	}
>  
>  	return err;
>  

Uncompilable since 2007!

That argues more for removal than anything else.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches - Feb. 24, 2013, 11:29 p.m.
On Sun, 2013-02-24 at 15:23 -0800, Joe Perches wrote:
> On Mon, 2013-02-25 at 04:31 +0530, Syam Sidhardhan wrote:
> > Fix missing () & { }
> []
> > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
> []
> > @@ -1829,10 +1831,11 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  		dev_err(&mac->pdev->dev, "register_netdev failed with error %d\n",
> >  			err);
> >  		goto out;
> > -	} else if netif_msg_probe(mac)
> > +	} else if (netif_msg_probe(mac)) {
> >  		printk(KERN_INFO "%s: PA Semi %s: intf %d, hw addr %pM\n",
> >  		       dev->name, mac->type == MAC_TYPE_GMAC ? "GMAC" : "XAUI",
> >  		       mac->dma_if, dev->dev_addr);
> > +	}
> >  
> >  	return err;
> >  
> 
> Uncompilable since 2007!
> That argues more for removal than anything else.

Nevermind. Not true.
netif_msg_probe is already surrounded by parens.
It's just stylistic ugly.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson - Feb. 25, 2013, 12:51 a.m.
On Sun, Feb 24, 2013 at 3:01 PM, Syam Sidhardhan <syamsidhardh@gmail.com> wrote:
> Fix missing () & { }
>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>

Acked-by: Olof Johansson <olof@lixom.net>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson - Feb. 25, 2013, 12:53 a.m.
On Sun, Feb 24, 2013 at 3:29 PM, Joe Perches <joe@perches.com> wrote:
> On Sun, 2013-02-24 at 15:23 -0800, Joe Perches wrote:
>> On Mon, 2013-02-25 at 04:31 +0530, Syam Sidhardhan wrote:
>> > Fix missing () & { }
>> []
>> > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
>> []
>> > @@ -1829,10 +1831,11 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> >             dev_err(&mac->pdev->dev, "register_netdev failed with error %d\n",
>> >                     err);
>> >             goto out;
>> > -   } else if netif_msg_probe(mac)
>> > +   } else if (netif_msg_probe(mac)) {
>> >             printk(KERN_INFO "%s: PA Semi %s: intf %d, hw addr %pM\n",
>> >                    dev->name, mac->type == MAC_TYPE_GMAC ? "GMAC" : "XAUI",
>> >                    mac->dma_if, dev->dev_addr);
>> > +   }
>> >
>> >     return err;
>> >
>>
>> Uncompilable since 2007!
>> That argues more for removal than anything else.

This driver is still used by several people running upstream kernels,
including myself. Removal is out of the question.

> Nevermind. Not true.
> netif_msg_probe is already surrounded by parens.
> It's just stylistic ugly.

Yeah, not sure how that one made it through way back then. Worth
fixing so it doesn't bite later if netif_msg_probe() is changed.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Feb. 25, 2013, 1:13 a.m.
From: Syam Sidhardhan <syamsidhardh@gmail.com>
Date: Mon, 25 Feb 2013 04:31:19 +0530

> -		} else
> +		} else {
>  			freed = 2;
> +		}

Single line statements do not need braces.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches - Feb. 25, 2013, 1:23 a.m.
On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
> From: Syam Sidhardhan <syamsidhardh@gmail.com>
> Date: Mon, 25 Feb 2013 04:31:19 +0530
> 
> > -             } else
> > +             } else {
> >                       freed = 2;
> > +             }
> 
> Single line statements do not need braces.

I guess you didn't see the preceding block with braces.

CodingStyle:

This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Feb. 25, 2013, 1:52 a.m.
From: Joe Perches <joe@perches.com>
Date: Sun, 24 Feb 2013 17:23:29 -0800

> On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
>> From: Syam Sidhardhan <syamsidhardh@gmail.com>
>> Date: Mon, 25 Feb 2013 04:31:19 +0530
>> 
>> > -             } else
>> > +             } else {
>> >                       freed = 2;
>> > +             }
>> 
>> Single line statements do not need braces.
> 
> I guess you didn't see the preceding block with braces.
> 
> CodingStyle:
> 
> This does not apply if only one branch of a conditional statement is a single
> statement; in the latter case use braces in both branches:

I did, and I don't think it's necessary in this case regardless
of what CodingStyle says, do you really honestly think it increases
readability enough to justify those two wasted vertical lines of
screen space?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches - Feb. 25, 2013, 2:11 a.m.
On Sun, 2013-02-24 at 20:52 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sun, 24 Feb 2013 17:23:29 -0800
> 
> > On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
> >> From: Syam Sidhardhan <syamsidhardh@gmail.com>
> >> Date: Mon, 25 Feb 2013 04:31:19 +0530
> >> 
> >> > -             } else
> >> > +             } else {
> >> >                       freed = 2;
> >> > +             }
> >> 
> >> Single line statements do not need braces.
> > 
> > I guess you didn't see the preceding block with braces.
> > 
> > CodingStyle:
> > 
> > This does not apply if only one branch of a conditional statement is a single
> > statement; in the latter case use braces in both branches:
> 
> I did, and I don't think it's necessary in this case regardless
> of what CodingStyle says, do you really honestly think it increases
> readability enough to justify those two wasted vertical lines of
> screen space?
> 

One additional line not two.

You've mentioned it yourself in the past as something
you seem to prefer.

http://markmail.org/message/go7z57ztnl2nivpz#query:+page:1+mid:tgpy4tricd5yqktp+state:results

and I don't much care except for consistency's sake.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Feb. 25, 2013, 2:22 a.m.
From: Joe Perches <joe@perches.com>
Date: Sun, 24 Feb 2013 18:11:09 -0800

> One additional line not two.
> 
> You've mentioned it yourself in the past as something
> you seem to prefer.
> 
> http://markmail.org/message/go7z57ztnl2nivpz#query:+page:1+mid:tgpy4tricd5yqktp+state:results
> 
> and I don't much care except for consistency's sake.

Fair enough, I'll apply this patch, thanks Joe.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
index 0be5844..b1cfbb7 100644
--- a/drivers/net/ethernet/pasemi/pasemi_mac.c
+++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
@@ -579,8 +579,9 @@  static void pasemi_mac_free_tx_resources(struct pasemi_mac *mac)
 						(TX_RING_SIZE-1)].dma;
 			freed = pasemi_mac_unmap_tx_skb(mac, nfrags,
 							info->skb, dmas);
-		} else
+		} else {
 			freed = 2;
+		}
 	}
 
 	kfree(txring->ring_info);
@@ -808,8 +809,9 @@  static int pasemi_mac_clean_rx(struct pasemi_mac_rxring *rx,
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			skb->csum = (macrx & XCT_MACRX_CSUM_M) >>
 					   XCT_MACRX_CSUM_S;
-		} else
+		} else {
 			skb_checksum_none_assert(skb);
+		}
 
 		packets++;
 		tot_bytes += len;
@@ -1829,10 +1831,11 @@  pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev_err(&mac->pdev->dev, "register_netdev failed with error %d\n",
 			err);
 		goto out;
-	} else if netif_msg_probe(mac)
+	} else if (netif_msg_probe(mac)) {
 		printk(KERN_INFO "%s: PA Semi %s: intf %d, hw addr %pM\n",
 		       dev->name, mac->type == MAC_TYPE_GMAC ? "GMAC" : "XAUI",
 		       mac->dma_if, dev->dev_addr);
+	}
 
 	return err;