drivers/usb/usbip/vudc_rx.c:145: possible bad bitmask ?
Hello there, drivers/usb/usbip/vudc_rx.c:145:27: warning: result of ‘11 << 30’ requires 35 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=] Source code is urb_p->urb->pipe &= ~(11 << 30); Maybe better code urb_p->urb->pipe &= ~(11UL << 30); Regards David Binderman -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linux-4.2-rc1/drivers/usb/host/ohci-tmio.c:104: possible missing break ?
Hello there, [linux-4.2-rc1/drivers/usb/host/ohci-tmio.c:104]: (warning) Redundant bitwise operation on 'pm' in 'switch' statement. 'break;' missing? case 3: pm |= CCR_PM_USBPW3; case 2: pm |= CCR_PM_USBPW2; case 1: pm |= CCR_PM_USBPW1; Suggest add missing break. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: linux-4.2-rc1/drivers/usb/host/ohci-tmio.c:104: possible missing break ?
Hello there Alan, Instead of suggesting an incorrect fix, why don't you submit a patch that fixes this correctly? I have a 0.0% (approx) success rate at submitting patches, so it would be pointless to try again for the umpteenth time. Mind you, I'm not stopping you having a go. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: OHCI: fix bad #define in ohci-tmio.c
Hello there, An incorrect definition of CCR_PM_USBPW3 in ohci-tmio.c is a perennial source of invalid diagnoses from static scanners, such as in http://marc.info/?l=linux-usbm=143634574527641w=2. This patch fixes the definition. Sure the patch changes the value of one of the macros used in the switch statement, but the code still has fallthrough from one case to another. That's what the static analyser is complaining about, not the values of the macros. I''m not sure if this fallthrough is intentional or not. If it is, it might be better etiquette to have a comment nearby with the word fallthrough in it, to give the rest of us a clue as to the code's intention. Something like case 3: pm |= CCR_PM_USBPW3; /* fallthrough */ case 2: pm |= CCR_PM_USBPW2; /* fallthrough */ case 1: pm |= CCR_PM_USBPW1; /* fallthrough */ On the other hand, if someone has blundered, and so the fallthrough is unintended, then I'd expect to see code something like this: case 3: pm |= CCR_PM_USBPW3; break; case 2: pm |= CCR_PM_USBPW2; break; case 1: pm |= CCR_PM_USBPW1; break; Just an idea. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: OHCI: fix bad #define in ohci-tmio.c
Hello there, Are you certain of that? The analyzer output you posted says Redundant bitwise operation on 'pm' in 'switch' statement -- that redundant seems to refer to the fact that the macro values are the same. Agreed - the macro values being the same is triggering the static analyser error. My mistake. Sorry. There is a similar message the analyser produces for missing breaks which confused me. I''m not sure if this fallthrough is intentional or not. It is. It's a not-uncommon idiom. Agreed. Always IMHO wise to document it, though. Perhaps a better solution would be like this: case 1: pm |= CCR_PM_USBPW1; break; case 2: pm |= CCR_PM_USBPW1 | CCR_PM_USBPW2; break; case 3: pm |= CCR_PM_USBPW1 | CCR_PM_USBPW2 | CCR_PM_USBPW3; break; Looks best solution to me. Maybe the only minor style quibble I have is to add () around the rhs of the |=. Like pm |= (CCR_PM_USBPW1 | CCR_PM_USBPW2 | CCR_PM_USBPW3); This is okay with only three cases, but it starts to get unwieldy. Agreed. If more cases are to be added in the future, it might be worth inventing some new macros. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linux-4.1/drivers/usb/serial/io_edgeport.c: 7 * redundant conditions ?
Hello there,
1.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1049]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
if ((!edge_serial-is_epic) ||
((edge_serial-is_epic)
(edge_serial-epic_descriptor.Supports.IOSPChase))) {
Maybe
if ((!edge_serial-is_epic) ||
edge_serial-epic_descriptor.Supports.IOSPChase) {
2.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1064]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
if ((!edge_serial-is_epic) ||
((edge_serial-is_epic)
(edge_serial-epic_descriptor.Supports.IOSPClose))) {
3.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1615]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
More of the same at lines 1631, 2468, 2497, 2501
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
RE: linux-4.1/drivers/usb/serial/io_edgeport.c: 7 * redundant conditions ?
Hello there Johan, Yes, that is a redundant test. Thanks for the confirmation. Care to fix these instances and submit a proper patch that we can apply? My success rate with kernel patches is 0.0%, despite many attempts over the years. I am happy for someone else to claim the victory of submitting a proper patch. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in
usb/phy/phy-msm-usb.c:1109: possible missing break ?
Hello there, [linux-3.19-rc5/drivers/usb/phy/phy-msm-usb.c:1109] - [linux-3.19-rc5/drivers/usb/phy/phy-msm-usb.c:1112]: (warning) Variable 'chg_state' is reassigned a value before the old one has been used. 'break;' missing? Source code is case USB_CHG_STATE_SECONDARY_DONE: motg-chg_state = USB_CHG_STATE_DETECTED; case USB_CHG_STATE_DETECTED: Suggest code rework. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
gadget/net2272.c:2075: possible bad test ?
Hello there,
[linux-3.16-rc4/drivers/usb/gadget/net2272.c:2075]: (style) Boolean result is
used in bitwise operation. Clarify expression with parentheses.
if (!intcsr (1 NET2272_PCI_IRQ)) {
Maybe the programmer intended
if (!(intcsr (1 NET2272_PCI_IRQ))) {
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
host/ohci-tmio.c:104: possible novice coding error ?
Hello there,
[linux-3.16-rc4/drivers/usb/host/ohci-tmio.c:104]: (warning) Redundant bitwise
operation on 'pm' in 'switch' statement. 'break;' missing?
switch (ohci-num_ports) {
default:
dev_err(dev-dev, Unsupported amount of ports: %d\n,
ohci-num_ports);
case 3:
pm |= CCR_PM_USBPW3;
case 2:
pm |= CCR_PM_USBPW2;
case 1:
pm |= CCR_PM_USBPW1;
}
Suggest add missing breaks.
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: gadget: gr_udc: Fix check for invalid number of microframes
Hello there,
The invalid value that should not be let through is 0x3 (4 transactions
per microframe).
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -1532,7 +1532,7 @@ static int gr_ep_enable(struct usb_ep *_ep,
%s mode: multiple trans./microframe not valid\n,
(mode == 2 ? Bulk : Control));
return -EINVAL;
- } else if (nt == 0x11) {
+ } else if (nt == 0x3) {
dev_err(dev-dev, Invalid value for trans./microframe\n);
return -EINVAL;
} else if ((nt + 1) * max buffer_size) {
--
1.7.10.4
When some value is out of band, I usually try to arrange that
the error message shows the value and, if possible, the band limits.
Suggest rework error message in call to dev_err. Maybe
dev_err(dev-dev, Invalid value '0x3' for transactions per microframe\n);
would be better.
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
usb/gadget/gr_udc.c:1528: possible bad test ?
Hello there,
[linux-3.16-rc1/drivers/usb/gadget/gr_udc.c:1528] -
[linux-3.16-rc1/drivers/usb/gadget/gr_udc.c:1535]:
(style) Mismatching assignment and comparison, comparison 'nt==17' is always
false.
Source code is
nt = 0x3 (usb_endpoint_maxp(desc) 11);
buffer_size = GR_BUFFER_SIZE(epctrl);
if (nt (mode == 0 || mode == 2)) {
dev_err(dev-dev,
%s mode: multiple trans./microframe not valid\n,
(mode == 2 ? Bulk : Control));
return -EINVAL;
} else if (nt == 0x11) {
If nt has been ANDed with 0x3, then it can't have a value of 0x11.
Suggest code rework.
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: musb_dsps: fix res_name length
Hello there, sprintf(res_name, port%d-mode, id); Hence, res_name must be at least 11 characters long in order to store the name including the terminating '\0'. Your patch seems plausible if the number of instances is 9 or less: indeed there currently might only be 2, if I read the source code ok. Parameter id is if type u8, you might want to add a few more chars to res_name to accommodate possible future expansion up to 256. Just an idea, it doesn't have to happen. Regards David Binderman Reported-by: David Binderman dcb...@hotmail.com Signed-off-by: Daniel Mack zon...@gmail.com --- drivers/usb/musb/musb_dsps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 444346e..425d893 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -458,7 +458,7 @@ static int __devinit dsps_create_musb_pdev(struct dsps_glue *glue, u8 id) struct platform_device *musb; struct resource *res; struct resource resources[2]; - char res_name[10]; + char res_name[11]; int ret, musbid; /* get memory resource */ -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: musb/musb_dsps.c:533]: (error) Buffer is accessed out of bounds
Hello there, I make that at least 11 chars into res_name. Suggest increase size. That is very true. Are you planning to release a patch for this? Sadly no. My success rate with patches is very low and I wouldn't know a good new size anyway. Mind you, if some enterprising person were to invent the patch, I wouldn't grumble ;- Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
usb/host/ohci-tmio.c:105: bad switch statement
Hello there,
I just ran the static analyser cppcheck over the source code of the
linux kernel version 3.6-rc1.
It said
linux-3.6-rc1/drivers/usb/host/ohci-tmio.c:105]: (warning) Redundant bitwise
operation on pm in switch
The source code is
switch (ohci-num_ports) {
default:
dev_err(dev-dev, Unsupported amount of ports: %d\n,
ohci-num_ports);
case 3:
pm |= CCR_PM_USBPW3;
case 2:
pm |= CCR_PM_USBPW2;
case 1:
pm |= CCR_PM_USBPW1;
}
Someone seems to have forgotten some break statements.
Suggest code rework.
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
