Message ID | 1361746879-29261-1-git-send-email-s.syam@samsung.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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
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
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
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
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
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
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
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
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
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;
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(-)