linux-kernel - Re: [PATCH] USB: remove dbg() usage in USB networking drivers

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, 19 Sep 2012 20:35:36 +0100
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Joe Perches <joe@...ches.com>
Cc:	netdev@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: remove dbg() usage in USB networking drivers

On Wed, Sep 19, 2012 at 10:53:32AM -0700, Joe Perches wrote:
> On Wed, 2012-09-19 at 18:13 +0100, Greg Kroah-Hartman wrote:
> > The dbg() USB macro is so old, it predates me.  The USB networking drivers are
> > the last hold-out using this macro, and we want to get rid of it, so replace
> > the usage of it with the proper netdev_dbg() or dev_dbg() (depending on the
> > context) calls.
> 
> There's a missing newline.

Oops, good catch.

> There are several dev_err(&intf-dev, ...) to dev_err(dev, ...)
> conversions.

That is because there is now a local variable for this, so I fixed the
whole function up to use it at the same time.

> Either those should be in a separate patch or you should mention
> those changes in the commit message.

Ok, let me resend...

> > diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c
> > index 26c5beb..6bf5672 100644
> > --- a/drivers/net/usb/catc.c
> > +++ b/drivers/net/usb/catc.c
> > @@ -236,7 +236,8 @@ static void catc_rx_done(struct urb *urb)
> >  	}
> >  
> >  	if (status) {
> > -		dbg("rx_done, status %d, length %d", status, urb->actual_length);
> > +		dev_dbg(&urb->dev->dev, "rx_done, status %d, length %d",
> > +			status, urb->actual_length);
> 
> newline
> 
> > @@ -774,7 +783,7 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id
> >  
> >  	if (usb_set_interface(usbdev,
> >  			intf->altsetting->desc.bInterfaceNumber, 1)) {
> > -                dev_err(&intf->dev, "Can't set altsetting 1.\n");
> > +		dev_err(dev, "Can't set altsetting 1.\n");
> 
> ?  Odd conversion.

We now have the local variable, so use it for all dev_* calls.

I've been doing that with the 30+ other dbg() cleanup patches in the usb
tree recently.

> > diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
> []
> > @@ -787,7 +787,8 @@ static void kaweth_usb_transmit_complete(struct urb *urb)
> >  
> >  	if (unlikely(status != 0))
> >  		if (status != -ENOENT)
> > -			dbg("%s: TX status %d.", kaweth->net->name, status);
> > +			dev_dbg(&urb->dev->dev, "%s: TX status %d.\n",
> > +				kaweth->net->name, status);
> 
> probably
> 			netdev_dbg(kaweth->net, "TX status %d\n", status);
> 
> [] 

It's the status of the urb that we want here, it is what failed, not the
netdev.  That's the way the rest of the kernel handles urb status debug
messages, so I'm trying to be consistant here.

> 
> > @@ -1003,36 +1005,37 @@ static int kaweth_probe(
> >  		const struct usb_device_id *id      /* from id_table */
> >  	)
> >  {
> > -	struct usb_device *dev = interface_to_usbdev(intf);
> > +	struct device *dev = &intf->dev;
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> 
> Miscellaneous renaming.

Consistant renaming :)

> > diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
> > index 28c4d51..aad7abe 100644
> > --- a/drivers/net/usb/net1080.c
> > +++ b/drivers/net/usb/net1080.c
> > @@ -156,11 +156,11 @@ static void nc_dump_registers(struct usbnet *dev)
> >  	u16	*vp = kmalloc(sizeof (u16));
> >  
> >  	if (!vp) {
> > -		dbg("no memory?");
> > +		netdev_dbg(dev->net, "no memory?\n");
> 
> could delete altogether.

Yeah, will do.

Thanks for the review, let me respin this...

greg k-h
--
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/