Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg

2020-01-08 Thread Malcolm Priestley
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 +, 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  # v5.3+
>>> Signed-off-by: Malcolm Priestley 
>>> ---
>>>  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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg

2020-01-06 Thread Quentin Deslandes
On 01/03/20 13:58:08, Dan Carpenter wrote:
> On Fri, Dec 20, 2019 at 09:14:59PM +, 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  # v5.3+
> > Signed-off-by: Malcolm Priestley 
> > ---
> >  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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg

2020-01-03 Thread Dan Carpenter
On Fri, Dec 20, 2019 at 09:14:59PM +, 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  # v5.3+
> Signed-off-by: Malcolm Priestley 
> ---
>  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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/5] staging: vt6656: Fix non zero logical return of, usb_control_msg

2019-12-20 Thread Malcolm Priestley
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  # v5.3+
Signed-off-by: Malcolm Priestley 
---
 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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel