* [PATCH v1] drivers: usb: Removes use of assignment in if condition
@ 2023-08-15 20:41 Atul Kumar Pant
2023-08-15 21:07 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Atul Kumar Pant @ 2023-08-15 20:41 UTC (permalink / raw)
To: gregkh, richard.leitner, wsa+renesas, mhocko, surenb
Cc: Atul Kumar Pant, linux-usb, linux-kernel, shuah
This patch fixes following checkpatch.pl issue:
ERROR: do not use assignment in if condition
Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
---
drivers/usb/core/devio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index e501a03d6c70..56899fed6bd4 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
}
}
+ intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
if (ps->dev->state != USB_STATE_CONFIGURED)
retval = -EHOSTUNREACH;
- else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
+ else if (!intf)
retval = -EINVAL;
else switch (ctl->ioctl_code) {
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] drivers: usb: Removes use of assignment in if condition
2023-08-15 20:41 [PATCH v1] drivers: usb: Removes use of assignment in if condition Atul Kumar Pant
@ 2023-08-15 21:07 ` Greg KH
2023-08-16 2:52 ` Atul Kumar Pant
2023-08-16 3:43 ` Atul Kumar Pant
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2023-08-15 21:07 UTC (permalink / raw)
To: Atul Kumar Pant
Cc: richard.leitner, wsa+renesas, mhocko, surenb, linux-usb,
linux-kernel, shuah
On Wed, Aug 16, 2023 at 02:11:41AM +0530, Atul Kumar Pant wrote:
> This patch fixes following checkpatch.pl issue:
> ERROR: do not use assignment in if condition
>
> Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> ---
> drivers/usb/core/devio.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index e501a03d6c70..56899fed6bd4 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> }
> }
>
> + intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
> if (ps->dev->state != USB_STATE_CONFIGURED)
> retval = -EHOSTUNREACH;
> - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> + else if (!intf)
Did you mean to change the logic here by doing the calculation always?
Does that change functionality?
The existing code is fine, running checkpatch on code outside of
drivers/staging/ or on new patches you are writing, is generally
discouraged as the code usually is older than checkpatch is :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] drivers: usb: Removes use of assignment in if condition
2023-08-15 21:07 ` Greg KH
@ 2023-08-16 2:52 ` Atul Kumar Pant
2023-08-16 14:43 ` Greg KH
2023-08-16 3:43 ` Atul Kumar Pant
1 sibling, 1 reply; 6+ messages in thread
From: Atul Kumar Pant @ 2023-08-16 2:52 UTC (permalink / raw)
To: Greg KH
Cc: richard.leitner, wsa+renesas, mhocko, surenb, linux-usb,
linux-kernel, shuah
On Tue, Aug 15, 2023 at 11:07:02PM +0200, Greg KH wrote:
> On Wed, Aug 16, 2023 at 02:11:41AM +0530, Atul Kumar Pant wrote:
> > This patch fixes following checkpatch.pl issue:
> > ERROR: do not use assignment in if condition
> >
> > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > ---
> > drivers/usb/core/devio.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > index e501a03d6c70..56899fed6bd4 100644
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> > }
> > }
> >
> > + intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
> > if (ps->dev->state != USB_STATE_CONFIGURED)
> > retval = -EHOSTUNREACH;
> > - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> > + else if (!intf)
>
> Did you mean to change the logic here by doing the calculation always?
> Does that change functionality?
>
> The existing code is fine, running checkpatch on code outside of
> drivers/staging/ or on new patches you are writing, is generally
> discouraged as the code usually is older than checkpatch is :)
Understood. One question though, should we fix issues like no space
before open parenthesis (e.g. if(val)) in the code that is outside
of /drivers/staging/ ?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] drivers: usb: Removes use of assignment in if condition
2023-08-15 21:07 ` Greg KH
2023-08-16 2:52 ` Atul Kumar Pant
@ 2023-08-16 3:43 ` Atul Kumar Pant
2023-08-16 14:42 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Atul Kumar Pant @ 2023-08-16 3:43 UTC (permalink / raw)
To: Greg KH
Cc: richard.leitner, wsa+renesas, mhocko, surenb, linux-usb,
linux-kernel, shuah
On Tue, Aug 15, 2023 at 11:07:02PM +0200, Greg KH wrote:
> On Wed, Aug 16, 2023 at 02:11:41AM +0530, Atul Kumar Pant wrote:
> > This patch fixes following checkpatch.pl issue:
> > ERROR: do not use assignment in if condition
> >
> > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > ---
> > drivers/usb/core/devio.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > index e501a03d6c70..56899fed6bd4 100644
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> > }
> > }
> >
> > + intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
> > if (ps->dev->state != USB_STATE_CONFIGURED)
> > retval = -EHOSTUNREACH;
> > - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> > + else if (!intf)
>
> Did you mean to change the logic here by doing the calculation always?
> Does that change functionality?
>
Now the calculation of interface will be done always, which can be
redundant. But this would not change the functionality.
> The existing code is fine, running checkpatch on code outside of
> drivers/staging/ or on new patches you are writing, is generally
> discouraged as the code usually is older than checkpatch is :)
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] drivers: usb: Removes use of assignment in if condition
2023-08-16 3:43 ` Atul Kumar Pant
@ 2023-08-16 14:42 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2023-08-16 14:42 UTC (permalink / raw)
To: Atul Kumar Pant
Cc: richard.leitner, wsa+renesas, mhocko, surenb, linux-usb,
linux-kernel, shuah
On Wed, Aug 16, 2023 at 09:13:13AM +0530, Atul Kumar Pant wrote:
> On Tue, Aug 15, 2023 at 11:07:02PM +0200, Greg KH wrote:
> > On Wed, Aug 16, 2023 at 02:11:41AM +0530, Atul Kumar Pant wrote:
> > > This patch fixes following checkpatch.pl issue:
> > > ERROR: do not use assignment in if condition
> > >
> > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > > ---
> > > drivers/usb/core/devio.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > > index e501a03d6c70..56899fed6bd4 100644
> > > --- a/drivers/usb/core/devio.c
> > > +++ b/drivers/usb/core/devio.c
> > > @@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> > > }
> > > }
> > >
> > > + intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
> > > if (ps->dev->state != USB_STATE_CONFIGURED)
> > > retval = -EHOSTUNREACH;
> > > - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> > > + else if (!intf)
> >
> > Did you mean to change the logic here by doing the calculation always?
> > Does that change functionality?
> >
> Now the calculation of interface will be done always, which can be
> redundant. But this would not change the functionality.
Yes, but it is now redundant, so this change isn't really needed, the
original code is "more correct", please leave it as-is.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] drivers: usb: Removes use of assignment in if condition
2023-08-16 2:52 ` Atul Kumar Pant
@ 2023-08-16 14:43 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2023-08-16 14:43 UTC (permalink / raw)
To: Atul Kumar Pant
Cc: richard.leitner, wsa+renesas, mhocko, surenb, linux-usb,
linux-kernel, shuah
On Wed, Aug 16, 2023 at 08:22:17AM +0530, Atul Kumar Pant wrote:
> On Tue, Aug 15, 2023 at 11:07:02PM +0200, Greg KH wrote:
> > On Wed, Aug 16, 2023 at 02:11:41AM +0530, Atul Kumar Pant wrote:
> > > This patch fixes following checkpatch.pl issue:
> > > ERROR: do not use assignment in if condition
> > >
> > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > > ---
> > > drivers/usb/core/devio.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > > index e501a03d6c70..56899fed6bd4 100644
> > > --- a/drivers/usb/core/devio.c
> > > +++ b/drivers/usb/core/devio.c
> > > @@ -2333,9 +2333,10 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
> > > }
> > > }
> > >
> > > + intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
> > > if (ps->dev->state != USB_STATE_CONFIGURED)
> > > retval = -EHOSTUNREACH;
> > > - else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
> > > + else if (!intf)
> >
> > Did you mean to change the logic here by doing the calculation always?
> > Does that change functionality?
> >
> > The existing code is fine, running checkpatch on code outside of
> > drivers/staging/ or on new patches you are writing, is generally
> > discouraged as the code usually is older than checkpatch is :)
>
> Understood. One question though, should we fix issues like no space
> before open parenthesis (e.g. if(val)) in the code that is outside
> of /drivers/staging/ ?
Only if the maintainer of that subsystem says it is ok to do so.
good luck!
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-16 14:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-15 20:41 [PATCH v1] drivers: usb: Removes use of assignment in if condition Atul Kumar Pant
2023-08-15 21:07 ` Greg KH
2023-08-16 2:52 ` Atul Kumar Pant
2023-08-16 14:43 ` Greg KH
2023-08-16 3:43 ` Atul Kumar Pant
2023-08-16 14:42 ` Greg KH
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).