netdev - Re: [PATCHv4 4/9] macb: convert printk to netdev

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:	Wed, 09 Nov 2011 05:10:47 -0800
From:	Joe Perches <joe@...ches.com>
To:	Jamie Iles <jamie@...ieiles.com>
Cc:	netdev@...r.kernel.org, arnd@...db.de
Subject: Re: [PATCHv4 4/9] macb: convert printk to netdev_ and friends

On Tue, 2011-11-08 at 14:13 +0000, Jamie Iles wrote:
> macb is already using the dev_dbg() and friends helpers so use netdev_()
> along with a pr_fmt() definition to make the printing a little cleaner.
trivia...

> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
[]
> @@ -8,6 +8,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#define pr_fmt(fmt) "macb: " fmt

I think this is better as
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> @@ -176,11 +177,11 @@ static void macb_handle_link_change(struct net_device *dev)
>  
>  	if (status_change) {
>  		if (phydev->link)
> -			printk(KERN_INFO "%s: link up (%d/%s)\n",
> -			       dev->name, phydev->speed,
> -			       DUPLEX_FULL == phydev->duplex ? "Full":"Half");
> +			netdev_info(dev, "link up (%d/%s)\n", phydev->speed,
> +				    DUPLEX_FULL == phydev->duplex ?
> +				    "Full" : "Half");

Couple of very trivial style things here.
I think this is better as var == const and I also try
to keep arguments on a single line where possible and
when not possible, more arguments after the format to
seoarate lines.

			netdev_info(dev, "link up (%d/%s)\n",
				    phydev->speed,
				    phydev->duplex == DUPLEX_FULL ?
				    "Full" : "Half");
[]
> @@ -625,12 +624,11 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  #ifdef DEBUG
>  	int i;
> -	dev_dbg(&bp->pdev->dev,
> -		"start_xmit: len %u head %p data %p tail %p end %p\n",
> -		skb->len, skb->head, skb->data,
> -		skb_tail_pointer(skb), skb_end_pointer(skb));
> -	dev_dbg(&bp->pdev->dev,
> -		"data:");
> +	netdev_dbg(bp->dev,
> +		   "start_xmit: len %u head %p data %p tail %p end %p\n",
> +		   skb->len, skb->head, skb->data,
> +		   skb_tail_pointer(skb), skb_end_pointer(skb));
> +	netdev_dbg(bp->dev, "data:");
>  	for (i = 0; i < 16; i++)
>  		printk(" %02x", (unsigned int)skb->data[i]);
>  	printk("\n");

Maybe print_hex_dump

> @@ -1228,13 +1223,13 @@ static int __init macb_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, dev);
>  
> -	printk(KERN_INFO "%s: Atmel MACB at 0x%08lx irq %d (%pM)\n",
> -	       dev->name, dev->base_addr, dev->irq, dev->dev_addr);
> +	netdev_info(dev, "Atmel MACB at 0x%08lx irq %d (%pM)\n",
> +		dev->base_addr, dev->irq, dev->dev_addr);
>  
>  	phydev = bp->phy_dev;
> -	printk(KERN_INFO "%s: attached PHY driver [%s] "
> -		"(mii_bus:phy_addr=%s, irq=%d)\n", dev->name,
> -		phydev->drv->name, dev_name(&phydev->dev), phydev->irq);
> +	netdev_info(dev, "attached PHY driver [%s] "
> +		    "(mii_bus:phy_addr=%s, irq=%d)\n", phydev->drv->name,
> +		    dev_name(&phydev->dev), phydev->irq);

Coalescing formats can also be done to make
it slightly easier to grep.

	netdev_info(dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
		    phydev->drv->name, dev_name(&phydev->dev), phydev->irq);

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html