| Message ID | 4d3fb70f-bf2f-96cc-a8fb-1ef658a24920@omp.ru |
|---|---|
| State | New |
| Headers | show |
| Series | usb: storage: freecom: drop useless assignment in init_freecom() | expand |
On Wed, Nov 29, 2023 at 11:43:50PM +0300, Sergey Shtylyov wrote: > In init_freecom(), the results of usb_stor_control_msg() calls are stored > in the local variable and then printed out by usb_stor_dbg() (if enabled), > except for the 1st call, the result of which is completely ignored. Drop > the useless assignment. Instead, you should check the return value and handle it properly, don't just drop the checking entirely, that's not good. thanks, greg k-h
On 12/4/23 10:04 AM, Greg Kroah-Hartman wrote: [...] >> In init_freecom(), the results of usb_stor_control_msg() calls are stored >> in the local variable and then printed out by usb_stor_dbg() (if enabled), >> except for the 1st call, the result of which is completely ignored. Drop >> the useless assignment. > > Instead, you should check the return value and handle it properly, don't > just drop the checking entirely, that's not good. Hmm... I wonder if you'd actually read the patch... I'm not dropping any checking because there's none, even at the further call sites of usb_stor_control_msg() -- the most init_freecom() currently is doing is printing out the result of the calls... > thanks, > > greg k-h MBR, Sergey
On 12/7/23 7:16 PM, Sergey Shtylyov wrote: [...] >>> In init_freecom(), the results of usb_stor_control_msg() calls are stored >>> in the local variable and then printed out by usb_stor_dbg() (if enabled), >>> except for the 1st call, the result of which is completely ignored. Drop >>> the useless assignment. >> >> Instead, you should check the return value and handle it properly, don't >> just drop the checking entirely, that's not good. > > Hmm... I wonder if you'd actually read the patch... > I'm not dropping any checking because there's none, even at the further > call sites of usb_stor_control_msg() -- the most init_freecom() currently > is doing is printing out the result of the calls... Alan, haven't heard your opinion on this patch... What do you think? [...] MBR, Sergey
Index: usb/drivers/usb/storage/freecom.c =================================================================== --- usb.orig/drivers/usb/storage/freecom.c +++ usb/drivers/usb/storage/freecom.c @@ -446,7 +446,7 @@ static int init_freecom(struct us_data * * all our packets. No need to allocate any extra buffer space. */ - result = usb_stor_control_msg(us, us->recv_ctrl_pipe, + usb_stor_control_msg(us, us->recv_ctrl_pipe, 0x4c, 0xc0, 0x4346, 0x0, buffer, 0x20, 3*HZ); buffer[32] = '\0'; usb_stor_dbg(us, "String returned from FC init is: %s\n", buffer);
In init_freecom(), the results of usb_stor_control_msg() calls are stored in the local variable and then printed out by usb_stor_dbg() (if enabled), except for the 1st call, the result of which is completely ignored. Drop the useless assignment. Found by Linux Verification Center (linuxtesting.org) with the Svace static analysis tool. Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- This patch is against the 'usb-next' branch of Greg KH's 'usb.git' repo... drivers/usb/storage/freecom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)