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 <[email protected]> >> --- >> 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. Thanks, Milan. Thanks, Milan.
