* [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg
@ 2019-12-20 21:14 Malcolm Priestley
2020-01-03 10:58 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Malcolm Priestley @ 2019-12-20 21:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: devel
Starting with commit 59608cb1de1856
("staging: vt6656: clean function's error path in usbpipe.c")
the usb control functions have returned errors throughout driver
with only logical variable checking.
However, usb_control_msg return the amount of bytes transferred
this means that normal operation causes errors.
Correct the return function so only return zero when transfer
is successful.
Cc: stable <stable@vger.kernel.org> # v5.3+
Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
---
drivers/staging/vt6656/usbpipe.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
index d3304df6bd53..488ebd98773d 100644
--- a/drivers/staging/vt6656/usbpipe.c
+++ b/drivers/staging/vt6656/usbpipe.c
@@ -59,7 +59,9 @@ int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
kfree(usb_buffer);
- if (ret >= 0 && ret < (int)length)
+ if (ret == (int)length)
+ ret = 0;
+ else
ret = -EIO;
end_unlock:
@@ -103,7 +105,9 @@ int vnt_control_in(struct vnt_private *priv, u8 request, u16 value,
kfree(usb_buffer);
- if (ret >= 0 && ret < (int)length)
+ if (ret == (int)length)
+ ret = 0;
+ else
ret = -EIO;
end_unlock:
--
2.24.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg
2019-12-20 21:14 [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg Malcolm Priestley
@ 2020-01-03 10:58 ` Dan Carpenter
2020-01-06 21:45 ` Quentin Deslandes
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-01-03 10:58 UTC (permalink / raw)
To: Malcolm Priestley; +Cc: devel, Greg Kroah-Hartman, Quentin Deslandes
On Fri, Dec 20, 2019 at 09:14:59PM +0000, Malcolm Priestley wrote:
> Starting with commit 59608cb1de1856
> ("staging: vt6656: clean function's error path in usbpipe.c")
> the usb control functions have returned errors throughout driver
> with only logical variable checking.
Use the Fixes tag for this.
Fixes: 59608cb1de18 ("staging: vt6656: clean function's error path in usbpipe.c")
12 digits to the hash. Add Quentin to the CC list.
>
> However, usb_control_msg return the amount of bytes transferred
> this means that normal operation causes errors.
>
> Correct the return function so only return zero when transfer
> is successful.
>
> Cc: stable <stable@vger.kernel.org> # v5.3+
> Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
> ---
> drivers/staging/vt6656/usbpipe.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
> index d3304df6bd53..488ebd98773d 100644
> --- a/drivers/staging/vt6656/usbpipe.c
> +++ b/drivers/staging/vt6656/usbpipe.c
> @@ -59,7 +59,9 @@ int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
>
> kfree(usb_buffer);
>
> - if (ret >= 0 && ret < (int)length)
> + if (ret == (int)length)
No need for this cast (no need in the original either).
> + ret = 0;
> + else
> ret = -EIO;
It would be better to preserve the error codes from usb_control_msg().
if (ret == length)
ret = 0;
else if (ret >= 0)
ret = -EIO;
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg
2020-01-03 10:58 ` Dan Carpenter
@ 2020-01-06 21:45 ` Quentin Deslandes
2020-01-08 21:55 ` Malcolm Priestley
0 siblings, 1 reply; 4+ messages in thread
From: Quentin Deslandes @ 2020-01-06 21:45 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Malcolm Priestley, Greg Kroah-Hartman, devel
On 01/03/20 13:58:08, Dan Carpenter wrote:
> On Fri, Dec 20, 2019 at 09:14:59PM +0000, Malcolm Priestley wrote:
> > Starting with commit 59608cb1de1856
> > ("staging: vt6656: clean function's error path in usbpipe.c")
> > the usb control functions have returned errors throughout driver
> > with only logical variable checking.
>
> Use the Fixes tag for this.
>
> Fixes: 59608cb1de18 ("staging: vt6656: clean function's error path in usbpipe.c")
>
> 12 digits to the hash. Add Quentin to the CC list.
>
> >
> > However, usb_control_msg return the amount of bytes transferred
> > this means that normal operation causes errors.
> >
> > Correct the return function so only return zero when transfer
> > is successful.
> >
> > Cc: stable <stable@vger.kernel.org> # v5.3+
> > Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
> > ---
> > drivers/staging/vt6656/usbpipe.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
> > index d3304df6bd53..488ebd98773d 100644
> > --- a/drivers/staging/vt6656/usbpipe.c
> > +++ b/drivers/staging/vt6656/usbpipe.c
> > @@ -59,7 +59,9 @@ int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
> >
> > kfree(usb_buffer);
> >
> > - if (ret >= 0 && ret < (int)length)
> > + if (ret == (int)length)
>
> No need for this cast (no need in the original either).
>
> > + ret = 0;
> > + else
> > ret = -EIO;
>
> It would be better to preserve the error codes from usb_control_msg().
>
> if (ret == length)
> ret = 0;
> else if (ret >= 0)
> ret = -EIO;
>
> regards,
> dan carpenter
>
Thanks for CC.
Nice catch. Dan is right, we should forward any error code from
usb_control_msg().
Regards,
Quentin Deslandes
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg
2020-01-06 21:45 ` Quentin Deslandes
@ 2020-01-08 21:55 ` Malcolm Priestley
0 siblings, 0 replies; 4+ messages in thread
From: Malcolm Priestley @ 2020-01-08 21:55 UTC (permalink / raw)
To: Quentin Deslandes, Dan Carpenter; +Cc: devel, Greg Kroah-Hartman
On 06/01/2020 21:45, Quentin Deslandes wrote:
> On 01/03/20 13:58:08, Dan Carpenter wrote:
>> On Fri, Dec 20, 2019 at 09:14:59PM +0000, Malcolm Priestley wrote:
>>> Starting with commit 59608cb1de1856
>>> ("staging: vt6656: clean function's error path in usbpipe.c")
>>> the usb control functions have returned errors throughout driver
>>> with only logical variable checking.
>>
>> Use the Fixes tag for this.
>>
>> Fixes: 59608cb1de18 ("staging: vt6656: clean function's error path in usbpipe.c")
>>
>> 12 digits to the hash. Add Quentin to the CC list.
>>
>>>
>>> However, usb_control_msg return the amount of bytes transferred
>>> this means that normal operation causes errors.
>>>
>>> Correct the return function so only return zero when transfer
>>> is successful.
>>>
>>> Cc: stable <stable@vger.kernel.org> # v5.3+
>>> Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
>>> ---
>>> drivers/staging/vt6656/usbpipe.c | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
>>> index d3304df6bd53..488ebd98773d 100644
>>> --- a/drivers/staging/vt6656/usbpipe.c
>>> +++ b/drivers/staging/vt6656/usbpipe.c
>>> @@ -59,7 +59,9 @@ int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
>>>
>>> kfree(usb_buffer);
>>>
>>> - if (ret >= 0 && ret < (int)length)
>>> + if (ret == (int)length)
>>
>> No need for this cast (no need in the original either).
>>
>>> + ret = 0;
>>> + else
>>> ret = -EIO;
>>
>> It would be better to preserve the error codes from usb_control_msg().
>>
>> if (ret == length)
>> ret = 0;
>> else if (ret >= 0)
>> ret = -EIO;
>>
>> regards,
>> dan carpenter
>>
>
> Thanks for CC.
>
> Nice catch. Dan is right, we should forward any error code from
> usb_control_msg().
This is already in linux-next.
So I will do a fix up patch by the end of this cycle.
Regards
Malcolm
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-08 21:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 21:14 [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg Malcolm Priestley
2020-01-03 10:58 ` Dan Carpenter
2020-01-06 21:45 ` Quentin Deslandes
2020-01-08 21:55 ` Malcolm Priestley
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).