Skip to contentSkip to conversation

coding style for drivers/staging/usbip

PATCH In review Cédric Cabessa 4 participants 10 messages Mar 19, 2014 – Mar 21, 2014 Staging

lore
Share & tools
lore

Cédric Cabessa

author
#0 lore raw

cover coding style for drivers/staging/usbip

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(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

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

Signed-off-by Cédric Cabessa <[email protected]>
---
 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(-)
drivers/staging/usbip/stub_tx.c +8 -8
index cd5326a..1622563 100644
@@ -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);
drivers/staging/usbip/usbip_common.c +4 -6
index 2a11233..184fa70 100644
@@ -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)
drivers/staging/usbip/vhci_sysfs.c +2 -2
index 82dd49f..e098032 100644
@@ -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);
Signature · 1 line
1.8.3.1
maintainer
#2 lore raw

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

Quoted 1 line · Cédric Cabessa
Signed-off-by Cédric Cabessa <[email protected]>

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

#3 lore raw

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

In reply to Cédric Cabessa

On Wed, 2014-03-19 at 23:04 +0100, Cédric Cabessa wrote:
[]

drivers/staging/usbip/usbip_common.c

Quoted 1 line
drivers/staging/usbip/usbip_common.c +0 -0

[]

drivers/staging/usbip/usbip_common.c static void usbip_dump_usb_ctrlrequest(struct usb_ctrlrequest *cmd) · line 184

Quoted 10 lines
drivers/staging/usbip/usbip_common.c +2 -2
@@ -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.

author
#4 lore raw

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

In reply to Cédric Cabessa
Signed-off-by Cédric Cabessa <[email protected]>
---
 drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)
drivers/staging/usbip/vhci_hcd.c +22 -11
index f690668..1e84577 100644
@@ -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");
Signature · 1 line
1.8.3.1
#5 lore raw

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

Hello.

Quoted 10 lines · Cédric Cabessa
Signed-off-by Cédric Cabessa <[email protected]>
---
  drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
  1 file changed, 22 insertions(+), 11 deletions(-)
drivers/staging/usbip/vhci_hcd.c +0 -0
index f690668..1e84577 100644

drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 275

Quoted 6 lines
drivers/staging/usbip/vhci_hcd.c +2 -1
@@ -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).

Quoted 12 lines
drivers/staging/usbip/vhci_hcd.c +3 -1
@@ -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;

drivers/staging/usbip/vhci_hcd.c static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state) · line 1077

Quoted 10 lines
drivers/staging/usbip/vhci_hcd.c +3 -1
@@ -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

#6 lore raw

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

Quoted 10 lines · Sergei Shtylyov
Quoted 10 lines
Signed-off-by Cédric Cabessa <[email protected]>
---
  drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
  1 file changed, 22 insertions(+), 11 deletions(-)
drivers/staging/usbip/vhci_hcd.c +0 -0
index f690668..1e84577 100644

[...]

Quoted 4 lines
Quoted 4 lines
drivers/staging/usbip/vhci_hcd.c +0 -0
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
 *pdev, pm_message_t state)
      spin_unlock(&the_controller->lock);
Quoted 12 lines
Quoted 9 lines
drivers/staging/usbip/vhci_hcd.c +3 -1
      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

#7 lore raw

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

In reply to Sergei Shtylyov
Quoted 2 lines · Sergei Shtylyov

drivers/staging/usbip/vhci_hcd.c

Quoted 1 line · Cédric Cabessa
drivers/staging/usbip/vhci_hcd.c +0 -0

[]

Quoted 11 lines

drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 275

Quoted 7 lines
drivers/staging/usbip/vhci_hcd.c +2 -1
@@ -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.

#8 lore raw

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

Quoted 14 lines · Joe Perches
Quoted 1 line

drivers/staging/usbip/vhci_hcd.c

Quoted 1 line
drivers/staging/usbip/vhci_hcd.c +0 -0

[]

Quoted 11 lines

drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 275

Quoted 7 lines
drivers/staging/usbip/vhci_hcd.c +2 -1
@@ -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).

Quoted 8 lines

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

#9 lore raw

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

In reply to Sergei Shtylyov
Quoted 10 lines · Sergei Shtylyov
Quoted 10 lines
Signed-off-by Cédric Cabessa <[email protected]>
---
  drivers/staging/usbip/vhci_hcd.c | 33 ++++++++++++++++++++++-----------
  1 file changed, 22 insertions(+), 11 deletions(-)
drivers/staging/usbip/vhci_hcd.c +0 -0
index f690668..1e84577 100644

[...]

Quoted 17 lines
Quoted 17 lines
drivers/staging/usbip/vhci_hcd.c +3 -1
@@ -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;
drivers/staging/usbip/vhci_hcd.c +0 -0
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
 *pdev, pm_message_t state)
      spin_unlock(&the_controller->lock);
Quoted 12 lines
Quoted 9 lines
drivers/staging/usbip/vhci_hcd.c +3 -1
      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