Re: [PATCH] staging: vt6656: remove unused variable
On Thu, May 16, 2019 at 11:39:51AM +0200, Greg Kroah-Hartman wrote: > On Thu, May 16, 2019 at 09:31:05AM +, Quentin Deslandes wrote: > > Fixed 'set but not used' warning message on a status variable. The > > called function returning the status code 'vnt_start_interrupt_urb()' > > clean up after itself and the caller function > > 'vnt_int_start_interrupt()' does not returns any value. > > > > Signed-off-by: Quentin Deslandes > > --- > > drivers/staging/vt6656/int.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c > > index 504424b19fcf..ac30ce72db5a 100644 > > --- a/drivers/staging/vt6656/int.c > > +++ b/drivers/staging/vt6656/int.c > > @@ -42,13 +42,12 @@ static const u8 fallback_rate1[5][5] = { > > void vnt_int_start_interrupt(struct vnt_private *priv) > > { > > unsigned long flags; > > - int status; > > > > dev_dbg(>usb->dev, ">Interrupt Polling Thread\n"); > > > > spin_lock_irqsave(>lock, flags); > > > > - status = vnt_start_interrupt_urb(priv); > > + vnt_start_interrupt_urb(priv); > > Shouldn't you fix this by erroring out if this fails? Why ignore the > errors? > > thanks, > > greg k-h I could, however 'vnt_start_interrupt_urb()' already call 'dev_dbg()' if it fails. Nothing is done after this debug call except returning an error code. 'vnt_int_start_interrupt()' should, IMHO, return a status code, but the original developer may have good reasons not to do so. Thank you, Quentin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: remove unused variable
On Thu, May 16, 2019 at 12:19:53PM +0200, Greg Kroah-Hartman wrote: > On Thu, May 16, 2019 at 09:50:38AM +, Quentin Deslandes wrote: > > On Thu, May 16, 2019 at 11:39:51AM +0200, Greg Kroah-Hartman wrote: > > > On Thu, May 16, 2019 at 09:31:05AM +, Quentin Deslandes wrote: > > > > Fixed 'set but not used' warning message on a status variable. The > > > > called function returning the status code 'vnt_start_interrupt_urb()' > > > > clean up after itself and the caller function > > > > 'vnt_int_start_interrupt()' does not returns any value. > > > > > > > > Signed-off-by: Quentin Deslandes > > > > --- > > > > drivers/staging/vt6656/int.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c > > > > index 504424b19fcf..ac30ce72db5a 100644 > > > > --- a/drivers/staging/vt6656/int.c > > > > +++ b/drivers/staging/vt6656/int.c > > > > @@ -42,13 +42,12 @@ static const u8 fallback_rate1[5][5] = { > > > > void vnt_int_start_interrupt(struct vnt_private *priv) > > > > { > > > > unsigned long flags; > > > > - int status; > > > > > > > > dev_dbg(>usb->dev, ">Interrupt Polling Thread\n"); > > > > > > > > spin_lock_irqsave(>lock, flags); > > > > > > > > - status = vnt_start_interrupt_urb(priv); > > > > + vnt_start_interrupt_urb(priv); > > > > > > Shouldn't you fix this by erroring out if this fails? Why ignore the > > > errors? > > > > > > thanks, > > > > > > greg k-h > > > > I could, however 'vnt_start_interrupt_urb()' already call 'dev_dbg()' if > > it fails. Nothing is done after this debug call except returning an > > error code. > > Returning an error code is fine for that function. But then you have to > do something with that error. > > > 'vnt_int_start_interrupt()' should, IMHO, return a status code, but the > > original developer may have good reasons not to do so. > > I think that is the real problem that needs to be fixed here, don't > paper over the issue by ignoring the return value. > > thanks, > > greg k-h Thus I'll return an error value to handle this in the caller function and then send a v2. Thank you for your help, Quentin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: remove unused variable
On Thu, May 16, 2019 at 09:50:38AM +, Quentin Deslandes wrote: > On Thu, May 16, 2019 at 11:39:51AM +0200, Greg Kroah-Hartman wrote: > > On Thu, May 16, 2019 at 09:31:05AM +, Quentin Deslandes wrote: > > > Fixed 'set but not used' warning message on a status variable. The > > > called function returning the status code 'vnt_start_interrupt_urb()' > > > clean up after itself and the caller function > > > 'vnt_int_start_interrupt()' does not returns any value. > > > > > > Signed-off-by: Quentin Deslandes > > > --- > > > drivers/staging/vt6656/int.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c > > > index 504424b19fcf..ac30ce72db5a 100644 > > > --- a/drivers/staging/vt6656/int.c > > > +++ b/drivers/staging/vt6656/int.c > > > @@ -42,13 +42,12 @@ static const u8 fallback_rate1[5][5] = { > > > void vnt_int_start_interrupt(struct vnt_private *priv) > > > { > > > unsigned long flags; > > > - int status; > > > > > > dev_dbg(>usb->dev, ">Interrupt Polling Thread\n"); > > > > > > spin_lock_irqsave(>lock, flags); > > > > > > - status = vnt_start_interrupt_urb(priv); > > > + vnt_start_interrupt_urb(priv); > > > > Shouldn't you fix this by erroring out if this fails? Why ignore the > > errors? > > > > thanks, > > > > greg k-h > > I could, however 'vnt_start_interrupt_urb()' already call 'dev_dbg()' if > it fails. Nothing is done after this debug call except returning an > error code. Returning an error code is fine for that function. But then you have to do something with that error. > 'vnt_int_start_interrupt()' should, IMHO, return a status code, but the > original developer may have good reasons not to do so. I think that is the real problem that needs to be fixed here, don't paper over the issue by ignoring the return value. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: remove unused variable
On Thu, May 16, 2019 at 09:31:05AM +, Quentin Deslandes wrote: > Fixed 'set but not used' warning message on a status variable. The > called function returning the status code 'vnt_start_interrupt_urb()' > clean up after itself and the caller function > 'vnt_int_start_interrupt()' does not returns any value. > > Signed-off-by: Quentin Deslandes > --- > drivers/staging/vt6656/int.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c > index 504424b19fcf..ac30ce72db5a 100644 > --- a/drivers/staging/vt6656/int.c > +++ b/drivers/staging/vt6656/int.c > @@ -42,13 +42,12 @@ static const u8 fallback_rate1[5][5] = { > void vnt_int_start_interrupt(struct vnt_private *priv) > { > unsigned long flags; > - int status; > > dev_dbg(>usb->dev, ">Interrupt Polling Thread\n"); > > spin_lock_irqsave(>lock, flags); > > - status = vnt_start_interrupt_urb(priv); > + vnt_start_interrupt_urb(priv); Shouldn't you fix this by erroring out if this fails? Why ignore the errors? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: remove unused variable
Fixed 'set but not used' warning message on a status variable. The called function returning the status code 'vnt_start_interrupt_urb()' clean up after itself and the caller function 'vnt_int_start_interrupt()' does not returns any value. Signed-off-by: Quentin Deslandes --- drivers/staging/vt6656/int.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c index 504424b19fcf..ac30ce72db5a 100644 --- a/drivers/staging/vt6656/int.c +++ b/drivers/staging/vt6656/int.c @@ -42,13 +42,12 @@ static const u8 fallback_rate1[5][5] = { void vnt_int_start_interrupt(struct vnt_private *priv) { unsigned long flags; - int status; dev_dbg(>usb->dev, ">Interrupt Polling Thread\n"); spin_lock_irqsave(>lock, flags); - status = vnt_start_interrupt_urb(priv); + vnt_start_interrupt_urb(priv); spin_unlock_irqrestore(>lock, flags); } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel