On Fri, 2 Jun 2017, Milan P. Gandhi wrote:
> On 06/01/2017 08:32 PM, Dan Carpenter wrote:
> > On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote:
> >> Simplify the check for return code of fcoe_if_init routine
> >> in fcoe_init function such that we could eliminate need for
> >> extra 'out_free' label.
> >>
> >> Signed-off-by: Milan P. Gandhi <mgandhi@xxxxxxxxxx>
> >> ---
> >> drivers/scsi/fcoe/fcoe.c | 10 ++++------
> >> 1 file changed, 4 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> >> index ea21e7b..fb2a4c9 100644
> >> --- a/drivers/scsi/fcoe/fcoe.c
> >> +++ b/drivers/scsi/fcoe/fcoe.c
> >> @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
> >> fcoe_dev_setup();
> >>
> >> rc = fcoe_if_init();
> >> - if (rc)
> >> - goto out_free;
> >> -
> >> - mutex_unlock(&fcoe_config_mutex);
> >> - return 0;
> >> + if (rc == 0) {
> >> + mutex_unlock(&fcoe_config_mutex);
> >> + return 0;
> >> + }
> >>
> >> -out_free:
> >> mutex_unlock(&fcoe_config_mutex);
> >
> > Gar... Stop! No1 Don't do this.
> >
> > Do failure handling, not success handling.
> >
> > People always think they should get creative with the last if statement
> > in a function. This leads to spaghetti code and it's confusing. Please
> > never do this again.
> >
> > The original is correct and the new code is bad rubbish code.
> >
> > regards,
> > dan carpenter
> >
> >
>
> Oops, my bad sir. Will keep this in mind.
Still, does the mutex_unlock really need to be duplicated?
julia
>
> Thanks,
> Milan.
>
>
> Thanks,
> Milan.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
