| Message ID | 20180915063645.31295-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: mptfusion: Remove unnecessary parentheses and simplify null checks | expand |
On Fri, Sep 14, 2018 at 11:36 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > Clang warns when multiple pairs of parentheses are used for a single > conditional statement. > > drivers/message/fusion/mptbase.c:338:11: warning: equality comparison > with extraneous parentheses [-Wparentheses-equality] > if ((ioc == NULL)) > ~~~~^~~~~~~ > drivers/message/fusion/mptbase.c:338:11: note: remove extraneous > parentheses around the comparison to silence this warning > if ((ioc == NULL)) > ~ ^ ~ > drivers/message/fusion/mptbase.c:338:11: note: use '=' to turn this > equality comparison into an assignment > if ((ioc == NULL)) > ^~ > = > drivers/message/fusion/mptbase.c:342:12: warning: equality comparison > with extraneous parentheses [-Wparentheses-equality] > if ((pdev == NULL)) > ~~~~~^~~~~~~ > drivers/message/fusion/mptbase.c:342:12: note: remove extraneous > parentheses around the comparison to silence this warning > if ((pdev == NULL)) > ~ ^ ~ > drivers/message/fusion/mptbase.c:342:12: note: use '=' to turn this > equality comparison into an assignment > if ((pdev == NULL)) > ^~ > = > 2 warnings generated. > > Remove them and while we're at it, simplify the NULL checks as '!var' is > used more than 'var == NULL'. > > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Nathan, Thanks for this patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/message/fusion/mptbase.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c > index dc1e43a02599..ba551d8dfba4 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) > MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; > struct pci_dev *pdev; > > - if ((ioc == NULL)) > + if (!ioc) > return -1; > > pdev = ioc->pcidev; > - if ((pdev == NULL)) > + if (!pdev) > return -1; > > pci_stop_and_remove_bus_device_locked(pdev); > -- > 2.18.0 >
Nathan, > Clang warns when multiple pairs of parentheses are used for a single > conditional statement. Applied to 4.20/scsi-queue. Thank you!
On Sat, Sep 15, 2018 at 8:38 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/message/fusion/mptbase.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c > index dc1e43a02599..ba551d8dfba4 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) > MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; > struct pci_dev *pdev; > > - if ((ioc == NULL)) > + if (!ioc) > return -1; > > pdev = ioc->pcidev; > - if ((pdev == NULL)) > + if (!pdev) > return -1; > > pci_stop_and_remove_bus_device_locked(pdev); > -- The exact same code also exists in drivers/scsi/mpt3sas/. I had a similar patch for both drivers, but saw that yours now got merged for fusion. If you don't mind, could you fix mpt3sas_base.c the same way? Arnd
On Tue, Oct 02, 2018 at 02:58:21PM +0200, Arnd Bergmann wrote: > On Sat, Sep 15, 2018 at 8:38 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > --- > > drivers/message/fusion/mptbase.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c > > index dc1e43a02599..ba551d8dfba4 100644 > > --- a/drivers/message/fusion/mptbase.c > > +++ b/drivers/message/fusion/mptbase.c > > @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) > > MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; > > struct pci_dev *pdev; > > > > - if ((ioc == NULL)) > > + if (!ioc) > > return -1; > > > > pdev = ioc->pcidev; > > - if ((pdev == NULL)) > > + if (!pdev) > > return -1; > > > > pci_stop_and_remove_bus_device_locked(pdev); > > -- > > > The exact same code also exists in drivers/scsi/mpt3sas/. I had a similar > patch for both drivers, but saw that yours now got merged for fusion. > > If you don't mind, could you fix mpt3sas_base.c the same way? > > Arnd Hi Arnd, I did sent a patch for this a little bit later: https://lore.kernel.org/lkml/20180920201002.23979-1-natechancellor@gmail.com/ I am still waiting for it to be merged. Thanks! Nathan
Nathan, >> The exact same code also exists in drivers/scsi/mpt3sas/. I had a similar >> patch for both drivers, but saw that yours now got merged for fusion. >> >> If you don't mind, could you fix mpt3sas_base.c the same way? >> >> Arnd > > Hi Arnd, > > I did sent a patch for this a little bit later: > https://lore.kernel.org/lkml/20180920201002.23979-1-natechancellor@gmail.com/ > > I am still waiting for it to be merged. I generally don't merge patches for drivers that are actively maintained unless the driver maintainer explicitly acks the change. However, this was trivial enough. Applied to 4.20/scsi-queue.
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c index dc1e43a02599..ba551d8dfba4 100644 --- a/drivers/message/fusion/mptbase.c +++ b/drivers/message/fusion/mptbase.c @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; struct pci_dev *pdev; - if ((ioc == NULL)) + if (!ioc) return -1; pdev = ioc->pcidev; - if ((pdev == NULL)) + if (!pdev) return -1; pci_stop_and_remove_bus_device_locked(pdev);
Clang warns when multiple pairs of parentheses are used for a single conditional statement. drivers/message/fusion/mptbase.c:338:11: warning: equality comparison with extraneous parentheses [-Wparentheses-equality] if ((ioc == NULL)) ~~~~^~~~~~~ drivers/message/fusion/mptbase.c:338:11: note: remove extraneous parentheses around the comparison to silence this warning if ((ioc == NULL)) ~ ^ ~ drivers/message/fusion/mptbase.c:338:11: note: use '=' to turn this equality comparison into an assignment if ((ioc == NULL)) ^~ = drivers/message/fusion/mptbase.c:342:12: warning: equality comparison with extraneous parentheses [-Wparentheses-equality] if ((pdev == NULL)) ~~~~~^~~~~~~ drivers/message/fusion/mptbase.c:342:12: note: remove extraneous parentheses around the comparison to silence this warning if ((pdev == NULL)) ~ ^ ~ drivers/message/fusion/mptbase.c:342:12: note: use '=' to turn this equality comparison into an assignment if ((pdev == NULL)) ^~ = 2 warnings generated. Remove them and while we're at it, simplify the NULL checks as '!var' is used more than 'var == NULL'. Reported-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/message/fusion/mptbase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)