[PATCH 0/2] coding style for drivers/staging/usbip

[PATCH 0/2] coding style for drivers/staging/usbip

[Cédric Cabessa]

Various coding style fixes for drivers/staging/usbip

Cédric Cabessa (2):
  coding style: fix quoted string split across lines
  coding style: fix line over 80 characters

 drivers/staging/usbip/stub_tx.c      | 16 ++++++++--------
 drivers/staging/usbip/usbip_common.c | 10 ++++------
 drivers/staging/usbip/vhci_hcd.c     | 33 ++++++++++++++++++++++-----------
 drivers/staging/usbip/vhci_sysfs.c   |  4 ++--
 4 files changed, 36 insertions(+), 27 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] coding style: fix quoted string split across lines

[Cédric Cabessa]

Signed-off-by: Cédric Cabessa <ced@ryick.net>
---
 drivers/staging/usbip/stub_tx.c      | 16 ++++++++--------
 drivers/staging/usbip/usbip_common.c | 10 ++++------
 drivers/staging/usbip/vhci_sysfs.c   |  4 ++--
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/usbip/stub_tx.c b/drivers/staging/usbip/stub_tx.c
index cd5326a..1622563 100644
--- a/drivers/staging/usbip/stub_tx.c
+++ b/drivers/staging/usbip/stub_tx.c
@@ -74,12 +74,12 @@ void stub_complete(struct urb *urb)
 		/* OK */
 		break;
 	case -ENOENT:
-		dev_info(&urb->dev->dev, "stopped by a call to usb_kill_urb() "
-			 "because of cleaning up a virtual connection\n");
+		dev_info(&urb->dev->dev,
+			 "stopped by a call to usb_kill_urb() because of cleaning up a virtual connection\n");
 		return;
 	case -ECONNRESET:
-		dev_info(&urb->dev->dev, "unlinked by a call to "
-			 "usb_unlink_urb()\n");
+		dev_info(&urb->dev->dev,
+			 "unlinked by a call to usb_unlink_urb()\n");
 		break;
 	case -EPIPE:
 		dev_info(&urb->dev->dev, "endpoint %d is stalled\n",
@@ -89,8 +89,9 @@ void stub_complete(struct urb *urb)
 		dev_info(&urb->dev->dev, "device removed?\n");
 		break;
 	default:
-		dev_info(&urb->dev->dev, "urb completion with non-zero status "
-			 "%d\n", urb->status);
+		dev_info(&urb->dev->dev,
+			 "urb completion with non-zero status %d\n",
+			 urb->status);
 		break;
 	}
 
@@ -228,8 +229,7 @@ static int stub_send_ret_submit(struct stub_device *sdev)
 
 			if (txsize != sizeof(pdu_header) + urb->actual_length) {
 				dev_err(&sdev->interface->dev,
-					"actual length of urb %d does not "
-					"match iso packet sizes %zu\n",
+					"actual length of urb %d does not match iso packet sizes %zu\n",
 					urb->actual_length,
 					txsize-sizeof(pdu_header));
 				kfree(iov);
diff --git a/drivers/staging/usbip/usbip_common.c b/drivers/staging/usbip/usbip_common.c
index 2a11233..184fa70 100644
--- a/drivers/staging/usbip/usbip_common.c
+++ b/drivers/staging/usbip/usbip_common.c
@@ -178,8 +178,8 @@ static void usbip_dump_usb_ctrlrequest(struct usb_ctrlrequest *cmd)
 	}
 
 	pr_debug("       ");
-	pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X) "
-		 "wLength(%04X) ", cmd->bRequestType, cmd->bRequest,
+	pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X) wLength(%04X) ",
+		 cmd->bRequestType, cmd->bRequest,
 		 cmd->wValue, cmd->wIndex, cmd->wLength);
 	pr_debug("\n       ");
 
@@ -290,8 +290,7 @@ void usbip_dump_header(struct usbip_header *pdu)
 
 	switch (pdu->base.command) {
 	case USBIP_CMD_SUBMIT:
-		pr_debug("USBIP_CMD_SUBMIT: "
-			 "x_flags %u x_len %u sf %u #p %d iv %d\n",
+		pr_debug("USBIP_CMD_SUBMIT: x_flags %u x_len %u sf %u #p %d iv %d\n",
 			 pdu->u.cmd_submit.transfer_flags,
 			 pdu->u.cmd_submit.transfer_buffer_length,
 			 pdu->u.cmd_submit.start_frame,
@@ -688,8 +687,7 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
 
 	if (total_length != urb->actual_length) {
 		dev_err(&urb->dev->dev,
-			"total length of iso packets %d not equal to actual "
-			"length of buffer %d\n",
+			"total length of iso packets %d not equal to actual length of buffer %d\n",
 			total_length, urb->actual_length);
 
 		if (ud->side == USBIP_STUB)
diff --git a/drivers/staging/usbip/vhci_sysfs.c b/drivers/staging/usbip/vhci_sysfs.c
index 82dd49f..e098032 100644
--- a/drivers/staging/usbip/vhci_sysfs.c
+++ b/drivers/staging/usbip/vhci_sysfs.c
@@ -47,8 +47,8 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
 	 * up /proc/net/{tcp,tcp6}. Also, a userland program may remember a
 	 * port number and its peer IP address.
 	 */
-	out += sprintf(out, "prt sta spd bus dev socket           "
-		       "local_busid\n");
+	out += sprintf(out,
+		       "prt sta spd bus dev socket           local_busid\n");
 
 	for (i = 0; i < VHCI_NPORTS; i++) {
 		struct vhci_device *vdev = port_to_vdev(i);
-- 
1.8.3.1

Re: [PATCH 1/2] coding style: fix quoted string split across lines

[Greg Kroah-Hartman]

On Wed, Mar 19, 2014 at 11:04:56PM +0100, Cédric Cabessa wrote:
> Signed-off-by: Cédric Cabessa <ced@ryick.net>

In the future, please pick a subject line that says what part of the
kernel you are changing.  For example, this patch would be:
	staging: usbip: fix quoted string split across lines

I'll fix these 2 up but in the future, please be more explicit.

thanks,

greg k-h

Re: [PATCH 1/2] coding style: fix quoted string split across lines

[Joe Perches]

On Wed, 2014-03-19 at 23:04 +0100, Cédric Cabessa wrote:
[]
> diff --git a/drivers/staging/usbip/usbip_common.c b/drivers/staging/usbip/usbip_common.c
[]
> @@ -178,8 +178,8 @@ static void usbip_dump_usb_ctrlrequest(struct usb_ctrlrequest *cmd)
>  	}
>  
>  	pr_debug("       ");
> -	pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X) "
> -		 "wLength(%04X) ", cmd->bRequestType, cmd->bRequest,
> +	pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X) wLength(%04X) ",
> +		 cmd->bRequestType, cmd->bRequest,
>  		 cmd->wValue, cmd->wIndex, cmd->wLength);
>  	pr_debug("\n       ");

While you didn't change it, these uses are broken.

pr_debug always starts a new line so the continuations
attempted in this block don't work.

Likely this was originally converted from some non pr_debug
mechanism without an understanding of how pr_debug varies
from a normal printf.

[PATCH 2/2] coding style: fix line over 80 characters

[Cédric Cabessa]

Signed-off-by: Cédric Cabessa <ced@ryick.net>
---
 drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index f690668..1e84577 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			}
 			break;
 		case USB_PORT_FEAT_POWER:
-			usbip_dbg_vhci_rh(" ClearPortFeature: USB_PORT_FEAT_POWER\n");
+			usbip_dbg_vhci_rh(
+				" ClearPortFeature: USB_PORT_FEAT_POWER\n");
 			dum->port_status[rhport] = 0;
 			dum->resuming = 0;
 			break;
 		case USB_PORT_FEAT_C_RESET:
-			usbip_dbg_vhci_rh(" ClearPortFeature: USB_PORT_FEAT_C_RESET\n");
+			usbip_dbg_vhci_rh(
+				" ClearPortFeature: USB_PORT_FEAT_C_RESET\n");
 			switch (dum->vdev[rhport].speed) {
 			case USB_SPEED_HIGH:
 				dum->port_status[rhport] |=
@@ -335,15 +337,17 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 			if (dum->vdev[rhport].ud.status ==
 			    VDEV_ST_NOTASSIGNED) {
-				usbip_dbg_vhci_rh(" enable rhport %d (status %u)\n",
-						  rhport,
-						  dum->vdev[rhport].ud.status);
+				usbip_dbg_vhci_rh(
+					" enable rhport %d (status %u)\n",
+					rhport,
+					dum->vdev[rhport].ud.status);
 				dum->port_status[rhport] |=
 					USB_PORT_STAT_ENABLE;
 			}
 		}
 		((__le16 *) buf)[0] = cpu_to_le16(dum->port_status[rhport]);
-		((__le16 *) buf)[1] = cpu_to_le16(dum->port_status[rhport] >> 16);
+		((__le16 *) buf)[1] =
+			cpu_to_le16(dum->port_status[rhport] >> 16);
 
 		usbip_dbg_vhci_rh(" GetPortStatus bye %x %x\n", ((u16 *)buf)[0],
 				  ((u16 *)buf)[1]);
@@ -355,10 +359,12 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	case SetPortFeature:
 		switch (wValue) {
 		case USB_PORT_FEAT_SUSPEND:
-			usbip_dbg_vhci_rh(" SetPortFeature: USB_PORT_FEAT_SUSPEND\n");
+			usbip_dbg_vhci_rh(
+				" SetPortFeature: USB_PORT_FEAT_SUSPEND\n");
 			break;
 		case USB_PORT_FEAT_RESET:
-			usbip_dbg_vhci_rh(" SetPortFeature: USB_PORT_FEAT_RESET\n");
+			usbip_dbg_vhci_rh(
+				" SetPortFeature: USB_PORT_FEAT_RESET\n");
 			/* if it's already running, disconnect first */
 			if (dum->port_status[rhport] & USB_PORT_STAT_ENABLE) {
 				dum->port_status[rhport] &=
@@ -530,7 +536,8 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 
 		case USB_REQ_GET_DESCRIPTOR:
 			if (ctrlreq->wValue == cpu_to_le16(USB_DT_DEVICE << 8))
-				usbip_dbg_vhci_hc("Not yet?:Get_Descriptor to device 0 (get max pipe size)\n");
+				usbip_dbg_vhci_hc(
+					"Not yet?:Get_Descriptor to device 0 (get max pipe size)\n");
 
 			if (vdev->udev)
 				usb_put_dev(vdev->udev);
@@ -539,7 +546,9 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 
 		default:
 			/* NOT REACHED */
-			dev_err(dev, "invalid request to devnum 0 bRequest %u, wValue %u\n", ctrlreq->bRequest,
+			dev_err(dev,
+				"invalid request to devnum 0 bRequest %u, wValue %u\n",
+				ctrlreq->bRequest,
 				ctrlreq->wValue);
 			ret =  -EINVAL;
 			goto no_need_xmit;
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
 	spin_unlock(&the_controller->lock);
 
 	if (connected > 0) {
-		dev_info(&pdev->dev, "We have %d active connection%s. Do not suspend.\n", connected, (connected == 1 ? "" : "s"));
+		dev_info(&pdev->dev,
+			 "We have %d active connection%s. Do not suspend.\n",
+			 connected, (connected == 1 ? "" : "s"));
 		ret =  -EBUSY;
 	} else {
 		dev_info(&pdev->dev, "suspend vhci_hcd");
-- 
1.8.3.1

Re: [PATCH 2/2] coding style: fix line over 80 characters

[Sergei Shtylyov]

Hello.

On 03/20/2014 01:04 AM, Cédric Cabessa wrote:

> Signed-off-by: Cédric Cabessa <ced@ryick.net>
> ---
>   drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
>   1 file changed, 22 insertions(+), 11 deletions(-)

> diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
> index f690668..1e84577 100644
> --- a/drivers/staging/usbip/vhci_hcd.c
> +++ b/drivers/staging/usbip/vhci_hcd.c
> @@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>   			}
>   			break;
>   		case USB_PORT_FEAT_POWER:
> -			usbip_dbg_vhci_rh(" ClearPortFeature: USB_PORT_FEAT_POWER\n");
> +			usbip_dbg_vhci_rh(
> +				" ClearPortFeature: USB_PORT_FEAT_POWER\n");

    Your version of scripts/checkpatch.pl seems outdated. It shouldn't 
complain about strings violating 80-column limit (and I've just verified it 
doesn't).

> @@ -539,7 +546,9 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
>
>   		default:
>   			/* NOT REACHED */
> -			dev_err(dev, "invalid request to devnum 0 bRequest %u, wValue %u\n", ctrlreq->bRequest,
> +			dev_err(dev,
> +				"invalid request to devnum 0 bRequest %u, wValue %u\n",
> +				ctrlreq->bRequest,
>   				ctrlreq->wValue);
>   			ret =  -EINVAL;
>   			goto no_need_xmit;
> @@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state)
>   	spin_unlock(&the_controller->lock);
>
>   	if (connected > 0) {
> -		dev_info(&pdev->dev, "We have %d active connection%s. Do not suspend.\n", connected, (connected == 1 ? "" : "s"));
> +		dev_info(&pdev->dev,
> +			 "We have %d active connection%s. Do not suspend.\n",
> +			 connected, (connected == 1 ? "" : "s"));
>   		ret =  -EBUSY;
>   	} else {
>   		dev_info(&pdev->dev, "suspend vhci_hcd");

    Hm, I don't see checkpatch.pl complaints about these either in the 
'usb-next' branch of Greg's tree.

WBR, Sergei

Re: [PATCH 2/2] coding style: fix line over 80 characters

[Joe Perches]

On Fri, 2014-03-21 at 00:34 +0300, Sergei Shtylyov wrote:
> On 03/20/2014 01:04 AM, Cédric Cabessa wrote:
> > diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
[]
> > @@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> >   			}
> >   			break;
> >   		case USB_PORT_FEAT_POWER:
> > -			usbip_dbg_vhci_rh(" ClearPortFeature: USB_PORT_FEAT_POWER\n");
> > +			usbip_dbg_vhci_rh(
> > +				" ClearPortFeature: USB_PORT_FEAT_POWER\n");
> 
>     Your version of scripts/checkpatch.pl seems outdated. It shouldn't 
> complain about strings violating 80-column limit (and I've just verified it 
> doesn't).

checkpatch complains about > 80 char lines for lines with
a function where the function name isn't understood to be
a logging use.

uspip_dbg_<foo> doesn't take the general form so if the
string at the EOL exceeds 80 chars, a message is emitted.

Long standalone strings on a single line do not get warnings.

Re: [PATCH 2/2] coding style: fix line over 80 characters

[Sergei Shtylyov]

On 03/20/2014 11:45 PM, Joe Perches wrote:

>>> diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
> []
>>> @@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>>>    			}
>>>    			break;
>>>    		case USB_PORT_FEAT_POWER:
>>> -			usbip_dbg_vhci_rh(" ClearPortFeature: USB_PORT_FEAT_POWER\n");
>>> +			usbip_dbg_vhci_rh(
>>> +				" ClearPortFeature: USB_PORT_FEAT_POWER\n");

>>      Your version of scripts/checkpatch.pl seems outdated. It shouldn't
>> complain about strings violating 80-column limit (and I've just verified it
>> doesn't).

> checkpatch complains about > 80 char lines for lines with
> a function where the function name isn't understood to be
> a logging use.

> uspip_dbg_<foo> doesn't take the general form so if the
> string at the EOL exceeds 80 chars, a message is emitted.

> Long standalone strings on a single line do not get warnings.

    OK, but there's still issue that the patch context doesn't correspond to 
what can be seen in Greg's tree. What I am seeing there is broken up string.

WBR, Sergei

Re: [PATCH 2/2] coding style: fix line over 80 characters

[Sergei Shtylyov]

On 03/21/2014 12:34 AM, Sergei Shtylyov wrote:

>> Signed-off-by: Cédric Cabessa <ced@ryick.net>
>> ---
>>   drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
>>   1 file changed, 22 insertions(+), 11 deletions(-)

>> diff --git a/drivers/staging/usbip/vhci_hcd.c
>> b/drivers/staging/usbip/vhci_hcd.c
>> index f690668..1e84577 100644
>> --- a/drivers/staging/usbip/vhci_hcd.c
>> +++ b/drivers/staging/usbip/vhci_hcd.c
[...]
>> @@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
>> *pdev, pm_message_t state)
>>       spin_unlock(&the_controller->lock);
>>
>>       if (connected > 0) {
>> -        dev_info(&pdev->dev, "We have %d active connection%s. Do not
>> suspend.\n", connected, (connected == 1 ? "" : "s"));
>> +        dev_info(&pdev->dev,
>> +             "We have %d active connection%s. Do not suspend.\n",
>> +             connected, (connected == 1 ? "" : "s"));
>>           ret =  -EBUSY;
>>       } else {
>>           dev_info(&pdev->dev, "suspend vhci_hcd");

>     Hm, I don't see checkpatch.pl complaints about these either in the
> 'usb-next' branch of Greg's tree.

    In fact, it does complain but says:

WARNING: quoted string split across lines

     So, it looks like your patch is against the wrong tree.

WBR, Sergei

Re: [PATCH 2/2] coding style: fix line over 80 characters

[Sergei Shtylyov]

On 03/21/2014 12:34 AM, Sergei Shtylyov wrote:

>> Signed-off-by: Cédric Cabessa <ced@ryick.net>
>> ---
>>   drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
>>   1 file changed, 22 insertions(+), 11 deletions(-)

>> diff --git a/drivers/staging/usbip/vhci_hcd.c
>> b/drivers/staging/usbip/vhci_hcd.c
>> index f690668..1e84577 100644
>> --- a/drivers/staging/usbip/vhci_hcd.c
>> +++ b/drivers/staging/usbip/vhci_hcd.c
[...]
>> @@ -539,7 +546,9 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct
>> urb *urb,
>>
>>           default:
>>               /* NOT REACHED */
>> -            dev_err(dev, "invalid request to devnum 0 bRequest %u, wValue
>> %u\n", ctrlreq->bRequest,
>> +            dev_err(dev,
>> +                "invalid request to devnum 0 bRequest %u, wValue %u\n",
>> +                ctrlreq->bRequest,
>>                   ctrlreq->wValue);
>>               ret =  -EINVAL;
>>               goto no_need_xmit;
>> @@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
>> *pdev, pm_message_t state)
>>       spin_unlock(&the_controller->lock);
>>
>>       if (connected > 0) {
>> -        dev_info(&pdev->dev, "We have %d active connection%s. Do not
>> suspend.\n", connected, (connected == 1 ? "" : "s"));
>> +        dev_info(&pdev->dev,
>> +             "We have %d active connection%s. Do not suspend.\n",
>> +             connected, (connected == 1 ? "" : "s"));
>>           ret =  -EBUSY;
>>       } else {
>>           dev_info(&pdev->dev, "suspend vhci_hcd");

>     Hm, I don't see checkpatch.pl complaints about these either in the
> 'usb-next' branch of Greg's tree.

    Sorry, I forgot about Greg's separate staging.git repo. The patch looks 
correct in this context.

WBR, Sergei