linux-kernel - Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1702282012460.2171@hadrien>
Date:   Tue, 28 Feb 2017 20:14:21 +0100 (CET)
From:   Julia Lawall <julia.lawall@...6.fr>
To:     SIMRAN SINGHAL <singhalsimran0@...il.com>
cc:     Joe Perches <joe@...ches.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        lustre-devel@...ts.lustre.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
        outreachy-kernel@...glegroups.com
Subject: Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove
 unnecessary else after return



On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:13 AM, Joe Perches <joe@...ches.com> wrote:
> > On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
> >> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches <joe@...ches.com> wrote:
> >> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >> > > This patch fixes the checkpatch warning that else is not generally
> >> > > useful after a break or return.
> >> >
> >> > checkpatch doesn't actually warn for this style
> >> >
> >> >         if (foo)
> >> >                 return bar;
> >> >         else
> >> >                 return baz;
> >> >
> >>
> >> ok, My bad
> >> so, I have to change commit message as checkpatch doesn't warn for this style.
> >
> > Perhaps better would be to leave them unchanged instead.
> >
> >> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> > []
> >> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct lnet_process_id id, __u32 ipaddr)
> >> > >
> >> > >       if (!count)
> >> > >               return -ENOENT;
> >> > > -     else
> >> > > -             return 0;
> >> > > +     return 0;
> >
> > There might be a case for this one.
> > error returns are generally in the form
> >
> > {
> >         [...]
> >
> >         err = func(...);
> >         if (err < 0)
> >                 return err;
> >
> >         return 0;
> > }
> Not sure, what's the problem in removing else as according to me
> there is no use of else.
>
> In this case if (if condition) does not satisfy then else condition will
> be satisfied and function will return 0.

I think that "there might be a case for" was a positive comment.  In any
case, it looks nicer to me if success is outside of a conditional and
failure is under a conditional.  Or to be more precise, Coccinelle bug
finding rules tend to work better if that property is respected.

julia

>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@...glegroups.com.
> To post to this group, send email to outreachy-kernel@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyP-BRPj4jEFgU%3DB_AV7E4-tJfgaXwwxRhUprLg_4gWQkg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>