Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH v2] usb: storage: initialize variable | From | Tom Rix <> | Date | Mon, 24 Aug 2020 14:31:01 -0700 |
| |
On 8/24/20 2:18 PM, Vito Caputo wrote: > On Mon, Aug 24, 2020 at 02:10:27PM -0700, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> clang static analysis reports this representative problem >> >> transport.c:495:15: warning: Assigned value is garbage or >> undefined >> length_left -= partial; >> ^ ~~~~~~~ >> partial is set only when usb_stor_bulk_transfer_sglist() >> is successful. >> >> So set partial on entry to 0. >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/usb/storage/transport.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c >> index 238a8088e17f..044429717dcc 100644 >> --- a/drivers/usb/storage/transport.c >> +++ b/drivers/usb/storage/transport.c >> @@ -414,6 +414,9 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe, >> { >> int result; >> >> + if (act_len) >> + *act_len = 0; >> + >> /* don't submit s-g requests during abort processing */ >> if (test_bit(US_FLIDX_ABORTING, &us->dflags)) >> return USB_STOR_XFER_ERROR; > At a glance this seems odd to me. If the caller insists on ignoring > the return value, shouldn't it just initialize partial to zero? > > In my experience it's generally frowned upon for functions to store > results in error paths.
Then maybe v1 is more appropriate.
Else i can spin a v3.
My preference is v1 as it doesn't add any runtime if-checks.
Tom
> Regards, > Vito Caputo >
| ![\](/images/icornerr.gif) |