All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] USB hub_probe: rework ugly goto-into-compound-statement
@ 2016-11-07 18:53 Eugene Korenevsky
  2016-11-08  6:50 ` Greg Kroah-Hartman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eugene Korenevsky @ 2016-11-07 18:53 UTC (permalink / raw)
  To: linux-kernel, linux-usb
  Cc: Greg Kroah-Hartman, Luiz Capitulino, Linus Torvalds,
	Chase Metzger, Alan Stern, Mathias Nyman, Lu Baolu, Oliver Neukum,
	Hans Yang

Rework smelling code (goto inside compound statement). Perhaps this is
legacy. Anyway such code is not appropriate for Linux kernel.

Signed-off-by: Eugene Korenevsky <ekorenevsky@gmail.com>
---
 drivers/usb/core/hub.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..7a20980 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1722,10 +1722,23 @@ static void hub_disconnect(struct usb_interface *intf)
 	kref_put(&hub->kref, hub_release);
 }
 
+static int hub_check_descriptor_sanity(struct usb_host_interface *desc)
+{
+	/* Some hubs have a subclass of 1, which AFAICT according to the */
+	/*  specs is not defined, but it works */
+	if (desc->desc.bInterfaceSubClass != 1 &&
+	    desc->desc.bInterfaceSubClass != 2)
+		return 0;
+	/* Multiple endpoints? What kind of mutant ninja-hub is this? */
+	if (desc->desc.bNumEndpoints != 1)
+		return 0;
+	/* If it's not an interrupt in endpoint, we'd better punt! */
+	return usb_endpoint_is_int_in(&desc->endpoint[0].desc);
+}
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *desc;
-	struct usb_endpoint_descriptor *endpoint;
 	struct usb_device *hdev;
 	struct usb_hub *hub;
 
@@ -1800,25 +1813,11 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	}
 #endif
 
-	/* Some hubs have a subclass of 1, which AFAICT according to the */
-	/*  specs is not defined, but it works */
-	if ((desc->desc.bInterfaceSubClass != 0) &&
-	    (desc->desc.bInterfaceSubClass != 1)) {
-descriptor_error:
+	if (!hub_check_descriptor_sanity(desc)) {
 		dev_err(&intf->dev, "bad descriptor, ignoring hub\n");
 		return -EIO;
 	}
 
-	/* Multiple endpoints? What kind of mutant ninja-hub is this? */
-	if (desc->desc.bNumEndpoints != 1)
-		goto descriptor_error;
-
-	endpoint = &desc->endpoint[0].desc;
-
-	/* If it's not an interrupt in endpoint, we'd better punt! */
-	if (!usb_endpoint_is_int_in(endpoint))
-		goto descriptor_error;
-
 	/* We found a hub */
 	dev_info(&intf->dev, "USB hub found\n");
 
@@ -1845,7 +1844,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
 		hub->quirk_check_port_auto_suspend = 1;
 
-	if (hub_configure(hub, endpoint) >= 0)
+	if (hub_configure(hub, &desc->endpoint[0].desc) >= 0)
 		return 0;
 
 	hub_disconnect(intf);
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-09  2:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-07 18:53 [PATCH v3 1/2] USB hub_probe: rework ugly goto-into-compound-statement Eugene Korenevsky
2016-11-08  6:50 ` Greg Kroah-Hartman
2016-11-08  6:51 ` Greg Kroah-Hartman
2016-11-09  2:36 ` [USB hub_probe] 384bb29a55: kmsg.hub#-#:#:bad_descriptor, ignoring_hub kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.