From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
Cc: "kernel-janitors@vger.kernel.org"
<kernel-janitors@vger.kernel.org>,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
Forest Bond <forest@alittletooquiet.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] staging: vt6656: remove unused variable
Date: Thu, 16 May 2019 10:19:53 +0000 [thread overview]
Message-ID: <20190516101953.GA22358@kroah.com> (raw)
In-Reply-To: <20190516095035.GA1692@qd-ubuntu>
On Thu, May 16, 2019 at 09:50:38AM +0000, 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 +0000, 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 <quentin.deslandes@itdev.co.uk>
> > > ---
> > > 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(&priv->usb->dev, "---->Interrupt Polling Thread\n");
> > >
> > > spin_lock_irqsave(&priv->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
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
Cc: "kernel-janitors@vger.kernel.org"
<kernel-janitors@vger.kernel.org>,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
Forest Bond <forest@alittletooquiet.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] staging: vt6656: remove unused variable
Date: Thu, 16 May 2019 12:19:53 +0200 [thread overview]
Message-ID: <20190516101953.GA22358@kroah.com> (raw)
In-Reply-To: <20190516095035.GA1692@qd-ubuntu>
On Thu, May 16, 2019 at 09:50:38AM +0000, 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 +0000, 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 <quentin.deslandes@itdev.co.uk>
> > > ---
> > > 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(&priv->usb->dev, "---->Interrupt Polling Thread\n");
> > >
> > > spin_lock_irqsave(&priv->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
next prev parent reply other threads:[~2019-05-16 10:19 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-16 9:31 [PATCH] staging: vt6656: remove unused variable Quentin Deslandes
2019-05-16 9:31 ` Quentin Deslandes
2019-05-16 9:39 ` Greg Kroah-Hartman
2019-05-16 9:39 ` Greg Kroah-Hartman
2019-05-16 9:50 ` Quentin Deslandes
2019-05-16 10:19 ` Greg Kroah-Hartman [this message]
2019-05-16 10:19 ` Greg Kroah-Hartman
2019-05-16 10:27 ` Quentin Deslandes
2019-05-16 11:22 ` [PATCH] staging: vt6656: returns error code on vnt_int_start_interrupt fail Quentin Deslandes
2019-05-16 11:22 ` Quentin Deslandes
2019-05-16 11:57 ` [PATCH v2] " Quentin Deslandes
2019-05-16 11:57 ` Quentin Deslandes
2019-05-17 7:31 ` Greg Kroah-Hartman
2019-05-17 7:31 ` Greg Kroah-Hartman
2019-05-17 7:53 ` [PATCH v3] " Quentin Deslandes
2019-05-17 7:53 ` Quentin Deslandes
2019-05-17 9:17 ` Greg Kroah-Hartman
2019-05-17 9:17 ` Greg Kroah-Hartman
2019-05-17 13:15 ` Quentin Deslandes
2019-05-17 13:29 ` Greg Kroah-Hartman
2019-05-17 13:29 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190516101953.GA22358@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=devel@driverdev.osuosl.org \
--cc=forest@alittletooquiet.net \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quentin.deslandes@itdev.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.