usb: misc: add a missing continue and refactor code
A message that is not in the archive (sent off-list or not yet received): 20170221153958.Horde.6KlAXD4A9Gyww1VviVYeCDh@gator4166.hostgator.com
PATCH 1/1 usb: misc: add a missing continue and refactor code
Code refactoring to make the flow easier to follow and add missing 'continue' for case USB_ENDPOINT_XFER_INT.
Addresses-Coverity-ID: 1248733
--- drivers/usb/misc/usbtest.c | 50 +++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 20 deletions(-)
drivers/usb/misc/usbtest.c +30 -20
@@ -124,6 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void try_intr (struct usb_host_endpoint *e, +struct usb_host_endpoint *int_in, +struct usb_host_endpoint *int_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!int_in) + int_in = e; + }else { +if (!int_out) + int_out = e; + } +} + +static inline void try_iso (struct usb_host_endpoint *e, +struct usb_host_endpoint *iso_in, +struct usb_host_endpoint *iso_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!iso_in) + iso_in = e; + }else { +if (!iso_out) + iso_out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) {
@@ -158,11 +184,12 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)break ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +try_intr (e, int_in, int_out); +continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; -/* FALLTHROUGH */ +try_iso (e, iso_in, iso_out); +/* fall through */ default :continue ; }
@@ -174,23 +201,6 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) out = e; }continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Signature · 1 line
2.5.0
Quoted 34 lines · Gustavo A. R. Silva
Code refactoring to make the flow easier to follow and add missing 'continue' for case
USB_ENDPOINT_XFER_INT.Addresses-Coverity-ID: 1248733
Cc Alan Stern <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 50 +++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 20 deletions(-)drivers/usb/misc/usbtest.c +16
-0@@ -124,6 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void try_intr (struct usb_host_endpoint *e, +struct usb_host_endpoint *int_in, +struct usb_host_endpoint *int_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!int_in) + int_in = e; + }else { +if (!int_out) + int_out = e; + } +} + +static inline void try_iso (struct usb_host_endpoint *e, +struct usb_host_endpoint *iso_in, +struct usb_host_endpoint *iso_out)
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 152
Quoted 10 lines
drivers/usb/misc/usbtest.c +10
-0+{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!iso_in) + iso_in = e; + }else { +if (!iso_out) + iso_out = e; + } +} +
This is not at all what I had in mind. First, it's incorrect (can you see why?). Second, by "inline" I meant moving the code to be actually in-line next to the conditional, not some place else in a separate subroutine (even if the subroutine is declared inline).
Also, the code for the USB_ENDPOINT_XFER_BULK case should look like the other two.
Alan Stern
Hi Alan,
Quoting Alan Stern <[email protected]>:
Quoted 46 lines · Gustavo A. R. Silva
Quoted 45 lines
Code refactoring to make the flow easier to follow and add missing 'continue' for case
USB_ENDPOINT_XFER_INT.Addresses-Coverity-ID: 1248733
Cc Alan Stern <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 50 +++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 20 deletions(-)drivers/usb/misc/usbtest.c +23
-0@@ -124,6 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) /*-------------------------------------------------------------------------*/ +static inline void try_intr (struct usb_host_endpoint *e, +struct usb_host_endpoint *int_in, +struct usb_host_endpoint *int_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!int_in) + int_in = e; + }else { +if (!int_out) + int_out = e; + } +} + +static inline void try_iso (struct usb_host_endpoint *e, +struct usb_host_endpoint *iso_in, +struct usb_host_endpoint *iso_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!iso_in) + iso_in = e; + }else { +if (!iso_out) + iso_out = e;
Quoted 9 lines
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 139
Quoted 3 lines
drivers/usb/misc/usbtest.c +3
-0+ } +} +This is not at all what I had in mind. First, it's incorrect (can you see why?). Second, by "inline" I meant moving the code to be actually in-line next to the conditional, not some place else in a separate subroutine (even if the subroutine is declared inline).
Interesting... let me double check.
I thought it would've been better to have separate inline subroutines for those "goto".
Quoted 3 lines
Also, the code for the
USB_ENDPOINT_XFER_BULKcase should look like the other two.
Do you mean a 'continue' instead of the 'break'?
Thanks for you comments.
--
Gustavo A. R. Silva
Quoting "Gustavo A. R. Silva" <[email protected]>:
Quoted 61 lines
Hi Alan,
Quoting Alan Stern <[email protected]>:
Quoted 45 lines · Gustavo A. R. Silva
Quoted 44 lines
Code refactoring to make the flow easier to follow and add missing 'continue' for case
USB_ENDPOINT_XFER_INT.Addresses-Coverity-ID: 1248733
Cc Alan Stern <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>---
drivers/usb/misc/usbtest.c| 50
+++++++++++++++++++++++++++-------------------1 file changed, 30 insertions(+), 20 deletions(-)drivers/usb/misc/usbtest.c +23
-0@@ -124,6 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) /*-------------------------------------------------------------------------*/ +static inline void try_intr (struct usb_host_endpoint *e, +struct usb_host_endpoint *int_in, +struct usb_host_endpoint *int_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!int_in) + int_in = e; + }else { +if (!int_out) + int_out = e; + } +} + +static inline void try_iso (struct usb_host_endpoint *e, +struct usb_host_endpoint *iso_in, +struct usb_host_endpoint *iso_out) +{ +if (usb_endpoint_dir_in (&e->desc)) { +if (!iso_in) + iso_in = e; + }else { +if (!iso_out) + iso_out = e;Quoted 9 lines
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 139
Quoted 3 lines
drivers/usb/misc/usbtest.c +3
-0+ } +} +This is not at all what I had in mind. First, it's incorrect (can you see why?). Second, by "inline" I meant moving the code to be actually in-line next to the conditional, not some place else in a separate subroutine (even if the subroutine is declared inline).
Interesting... let me double check.
Quoted 8 lines
I thought it would've been better to have separate inline subroutines for those "goto".
Quoted 3 lines
Also, the code for the
USB_ENDPOINT_XFER_BULKcase should look like the other two.Do you mean a 'continue' instead of the 'break'?
Oh I see, the following piece of code should be part of the USB_ENDPOINT_XFER_BULK case:
if (usb_endpoint_dir_in(&e->desc)) {
if (!in)
in = e;
} else {
if (!out)
out = e;
}continue;
--
Gustavo A. R. Silva
PATCH 1/1 usb: misc: add missing continue and refactor code
-Code refactoring to make the flow easier to follow. -Add missing 'continue' for case USB_ENDPOINT_XFER_INT.
Addresses-Coverity-ID: 1248733
--- drivers/usb/misc/usbtest.c | 68 +++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 37 deletions(-)
drivers/usb/misc/usbtest.c +31 -37
@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt; -struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp; +struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep; in = out =NULL ; iso_in = iso_out =NULL ;
@@ -150,47 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)* ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e; +int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e); +continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; -/* FALLTHROUGH */ +endpoint_update (edi, &iso_in, &iso_out, e); +/* fall through */ default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Signature · 1 line
2.5.0
Greg Kroah-Hartman
maintainerRe: [PATCH] usb: misc: add missing continue and refactor code
Review
Quoted 2 lines · Gustavo A. R. Silva
(quoted diff) +0
-2-Code refactoring to make the flow easier to follow. -Add missing 'continue' for case USB_ENDPOINT_XFER_INT.
Don't do multiple things in the same patch, please make these multiple patches. And do the "add missing continue" first, so it can be backported to other kernels easier please.
thanks,
greg k-h
Re: [PATCH] usb: misc: add missing continue and refactor code
Quoted 7 lines · Greg Kroah-Hartman
Review
Quoted 2 lines · Gustavo A. R. Silva
(quoted diff) +0
-2-Code refactoring to make the flow easier to follow. -Add missing 'continue' for case USB_ENDPOINT_XFER_INT.Don't do multiple things in the same patch, please make these multiple patches. And do the "add missing continue" first, so it can be backported to other kernels easier please.
Also, make sure your patch does not contain gratuitous whitespace changes.
Alan Stern
Re: [PATCH] usb: misc: add missing continue and refactor code
Quoting Alan Stern <[email protected]>:
Quoted 10 lines · Greg Kroah-Hartman
Quoted 8 lines · Gustavo A. R. Silva
Review
Quoted 2 lines
(quoted diff) +0
-2-Code refactoring to make the flow easier to follow. -Add missing 'continue' for case USB_ENDPOINT_XFER_INT.Don't do multiple things in the same patch, please make these multiple patches. And do the "add missing continue" first, so it can be backported to other kernels easier please.
OK, I will send a patchset shortly.
Quoted 3 lines
Also, make sure your patch does not contain gratuitous whitespace changes.
Does it have any?
I ran it through checkpatch.pl before sending it and didn't see any.
Thanks
--
Gustavo A. R. Silva
PATCH 1/2 usb: misc: add missing continue in switch
Add missing continue in switch.
Addresses-Coverity-ID: 1248733
--- drivers/usb/misc/usbtest.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/misc/usbtest.c +1 -0
@@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)case USB_ENDPOINT_XFER_INT :if (dev->info->intr) goto try_intr; +continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) goto try_iso;
Signature · 1 line
2.5.0
PATCH 2/2 usb: misc: refactor code
Code refactoring to make the flow easier to follow.
--- drivers/usb/misc/usbtest.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-)
drivers/usb/misc/usbtest.c +30 -37
@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt; -struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp; +struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep; in = out =NULL ; iso_in = iso_out =NULL ;
@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)* ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e; +int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e);continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; -/* FALLTHROUGH */ +endpoint_update (edi, &iso_in, &iso_out, e); +/* fall through */ default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Signature · 1 line
2.5.0
Re: [PATCH 2/2] usb: misc: refactor code
Quoted 36 lines · Gustavo A. R. Silva
Code refactoring to make the flow easier to follow.
Cc Alan Stern <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-)drivers/usb/misc/usbtest.c +14
-2@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt;
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 149
Quoted 10 lines
drivers/usb/misc/usbtest.c +6
-4-struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp; +struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;
What's the difference between these 6 lines you added and the 6 lines that were there originally?
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 152
Quoted 3 lines
drivers/usb/misc/usbtest.c +1
-1for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep;
And here?
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) · line 167
Quoted 9 lines
drivers/usb/misc/usbtest.c +1
-1in = out =NULL ; iso_in = iso_out =NULL ;@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)* ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e;
And here?
Quoted 11 lines
drivers/usb/misc/usbtest.c +5
-1+int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) · line 184
Quoted 10 lines
drivers/usb/misc/usbtest.c +3
-3if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e);continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; -/* FALLTHROUGH */ +endpoint_update (edi, &iso_in, &iso_out, e); +/* fall through */
Why change the comment?
Alan Stern
Quoted 32 lines
drivers/usb/misc/usbtest.c +0
-25default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Re: [PATCH 2/2] usb: misc: refactor code
Hi Alan,
Quoting Alan Stern <[email protected]>:
Quoted 46 lines · Gustavo A. R. Silva
Quoted 45 lines
Code refactoring to make the flow easier to follow.
Cc Alan Stern <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-)drivers/usb/misc/usbtest.c +15
-6@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) /*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt; -struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp;
Quoted 9 lines
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 149
Quoted 5 lines
drivers/usb/misc/usbtest.c +5
-0+struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;What's the difference between these 6 lines you added and the 6 lines that were there originally?
Yeah, well, certainly none at all. This is what happened: I created a local copy of my changes(this piece of code included) and fixed some issues in a previous patch, then I did a git revert and moved my changes back to the original file. During this process the tabs were replaced with spaces in the original file, then I had to add the tabs again and this was the resulting patch.
Quoted 6 lines
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 152
Quoted 3 lines
drivers/usb/misc/usbtest.c +1
-1for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep;And here?
Same as above.
Quoted 13 lines
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct · line 168
Quoted 10 lines
drivers/usb/misc/usbtest.c +1
-1in = out =NULL ; iso_in = iso_out =NULL ;@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, structusb_interface *intf) * ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e;And here?
Same as above.
Quoted 15 lines
Quoted 15 lines
drivers/usb/misc/usbtest.c +6
-2+int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e);continue ;
Quoted 9 lines
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) · line 184
Quoted 6 lines
drivers/usb/misc/usbtest.c +2
-2case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; -/* FALLTHROUGH */ +endpoint_update (edi, &iso_in, &iso_out, e); +/* fall through */ Why change the comment?
Oh, I based this change in the following comment to another patch I sent some weeks ago:
https://lkml.org/lkml/2017/2/10/293
It is due to Coding Style.
Quoted 26 lines
Alan Stern
Quoted 24 lines
drivers/usb/misc/usbtest.c +0
-21default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e;
Quoted 8 lines
Quoted 8 lines
drivers/usb/misc/usbtest.c +0
-4- }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out)goto found;
Thanks for your comments.
--
Gustavo A. R. Silva
PATCH v2 1/2 usb: misc: add missing continue in switch
Add missing continue in switch.
Addresses-Coverity-ID: 1248733
---
Changes in v2:
None.
drivers/usb/misc/usbtest.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/misc/usbtest.c +1 -0
@@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)case USB_ENDPOINT_XFER_INT :if (dev->info->intr) goto try_intr; +continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) goto try_iso;
Signature · 1 line
2.5.0
PATCH v2 2/2 usb: misc: refactor code
Code refactoring to make the flow easier to follow.
---
Changes in v2:
Remove unfruitful changes in previous patch.
Revert change to comment.
drivers/usb/misc/usbtest.c | 49 ++++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 28 deletions(-)
drivers/usb/misc/usbtest.c +21 -28
@@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) {
@@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)*/ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) {struct usb_host_endpoint *e; +int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e);continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; +endpoint_update (edi, &iso_in, &iso_out, e);/* FALLTHROUGH */ default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Signature · 1 line
2.5.0
Re: [PATCH v2 2/2] usb: misc: refactor code
Quoted 9 lines · Gustavo A. R. Silva
Code refactoring to make the flow easier to follow.
Cc Alan Stern <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>---
Changes in v2:
Remove unfruitful changes in previous patch.
Revert change to comment.
For both patches:
Quoted 84 lines
drivers/usb/misc/usbtest.c | 49 ++++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 28 deletions(-)drivers/usb/misc/usbtest.c +21
-28@@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) {@@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)*/ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) {struct usb_host_endpoint *e; +int edi; e = alt->endpoint + ep; + edi =usb_endpoint_dir_in (&e->desc); +switch (usb_endpoint_type (&e->desc)) {case USB_ENDPOINT_XFER_BULK : -break ; +endpoint_update (edi, &in, &out, e); +continue ;case USB_ENDPOINT_XFER_INT :if (dev->info->intr) - goto try_intr; +endpoint_update (edi, &int_in, &int_out, e);continue ;case USB_ENDPOINT_XFER_ISOC :if (dev->info->iso) - goto try_iso; +endpoint_update (edi, &iso_in, &iso_out, e);/* FALLTHROUGH */ default :continue ; } -if (usb_endpoint_dir_in (&e->desc)) { -if (!in) - in = e; - }else { -if (!out) - out = e; - } -continue ; -try_intr: -if (usb_endpoint_dir_in (&e->desc)) { -if (!int_in) - int_in = e; - }else { -if (!int_out) - int_out = e; - } -continue ; -try_iso: -if (usb_endpoint_dir_in (&e->desc)) { -if (!iso_in) - iso_in = e; - }else { -if (!iso_out) - iso_out = e; - } }if ((in && out) || iso_in || iso_out || int_in || int_out) goto found;
Re: [PATCH 2/2] usb: misc: refactor code
In reply to Gustavo A. R. SilvaHi,
Quoted 36 lines · Gustavo A. R. Silva
Code refactoring to make the flow easier to follow.
Cc Alan Stern <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-)drivers/usb/misc/usbtest.c +14
-2@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test)/*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt;
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 149
Quoted 10 lines
drivers/usb/misc/usbtest.c +6
-4-struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp; +struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;
unnecessary change
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 152
Quoted 4 lines
drivers/usb/misc/usbtest.c +1
-1for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep;
unnecessary change
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) · line 167
Quoted 9 lines
drivers/usb/misc/usbtest.c +1
-1in = out =NULL ; iso_in = iso_out =NULL ;@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)* ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e;
unnecessary change
Signature · 1 line
balbi
Re: [PATCH 2/2] usb: misc: refactor code
Hello,
Quoting Felipe Balbi <[email protected]>:
Quoted 63 lines
Hi,
Quoted 39 lines · Gustavo A. R. Silva
Code refactoring to make the flow easier to follow.
Cc Alan Stern <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/usb/misc/usbtest.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-)drivers/usb/misc/usbtest.c +14
-2@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) /*-------------------------------------------------------------------------*/ +static inline void endpoint_update (int edi, +struct usb_host_endpoint **in, +struct usb_host_endpoint **out, +struct usb_host_endpoint *e) +{ +if (edi) { +if (!*in) + *in = e; + }else { +if (!*out) + *out = e; + } +} +static int get_endpoints (struct usbtest_dev *dev,struct usb_interface *intf) { -int tmp; -struct usb_host_interface *alt;drivers/usb/misc/usbtest.c static struct usb_device · line 151
Quoted 10 lines
drivers/usb/misc/usbtest.c +6
-4-struct usb_host_endpoint *in, *out; -struct usb_host_endpoint *iso_in, *iso_out; -struct usb_host_endpoint *int_in, *int_out; -struct usb_device *udev; +int tmp; +struct usb_host_interface *alt; +struct usb_host_endpoint *in, *out; +struct usb_host_endpoint *iso_in, *iso_out; +struct usb_host_endpoint *int_in, *int_out; +struct usb_device *udev;unnecessary change
drivers/usb/misc/usbtest.c static struct usb_device *testdev_to_usbdev(struct usbtest_dev *test) · line 152
Quoted 4 lines
drivers/usb/misc/usbtest.c +1
-1for (tmp =0 ; tmp < intf->num_altsetting; tmp++) { -unsigned ep; +unsigned ep;unnecessary change
Quoted 1 line
Quoted 12 lines
drivers/usb/misc/usbtest.c get_endpoints(struct usbtest_dev *dev, struct · line 168
Quoted 9 lines
drivers/usb/misc/usbtest.c +1
-1in = out =NULL ; iso_in = iso_out =NULL ;@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, structusb_interface *intf) * ignore other endpoints and altsettings. */ for (ep =0 ; ep < alt->desc.bNumEndpoints; ep++) { -struct usb_host_endpoint *e; +struct usb_host_endpoint *e;unnecessary change
I already sent the version 2 of this patch: https://lkml.org/lkml/2017/4/3/856
Thanks
--
Gustavo A. R. Silva