* [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
2016-11-08 6:51 ` Greg Kroah-Hartman
0 siblings, 2 replies; 3+ 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] 3+ messages in thread
* Re: [PATCH v3 1/2] USB hub_probe: rework ugly goto-into-compound-statement
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
1 sibling, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2016-11-08 6:50 UTC (permalink / raw)
To: Eugene Korenevsky
Cc: linux-kernel, linux-usb, Luiz Capitulino, Linus Torvalds,
Chase Metzger, Alan Stern, Mathias Nyman, Lu Baolu,
Oliver Neukum, Hans Yang
On Mon, Nov 07, 2016 at 09:53:50PM +0300, Eugene Korenevsky wrote:
> 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(-)
You forgot to list what changed from v2 here :(
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/2] USB hub_probe: rework ugly goto-into-compound-statement
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
1 sibling, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2016-11-08 6:51 UTC (permalink / raw)
To: Eugene Korenevsky
Cc: linux-kernel, linux-usb, Luiz Capitulino, Linus Torvalds,
Chase Metzger, Alan Stern, Mathias Nyman, Lu Baolu,
Oliver Neukum, Hans Yang
On Mon, Nov 07, 2016 at 09:53:50PM +0300, Eugene Korenevsky wrote:
> 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);
Minor nit, put blank lines after each if statement to make it readable.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-08 6:51 UTC | newest]
Thread overview: 3+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).