* [PATCH] Eliminate extra 'out_free' label from fcoe_init function @ 2017-06-01 12:23 Milan P. Gandhi 2017-06-01 13:07 ` Johannes Thumshirn 2017-06-01 15:02 ` Dan Carpenter 0 siblings, 2 replies; 6+ messages in thread From: Milan P. Gandhi @ 2017-06-01 12:23 UTC (permalink / raw) To: linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman 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@redhat.com> --- 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); out_destroy: destroy_workqueue(fcoe_wq); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function 2017-06-01 12:23 [PATCH] Eliminate extra 'out_free' label from fcoe_init function Milan P. Gandhi @ 2017-06-01 13:07 ` Johannes Thumshirn 2017-06-01 15:02 ` Dan Carpenter 1 sibling, 0 replies; 6+ messages in thread From: Johannes Thumshirn @ 2017-06-01 13:07 UTC (permalink / raw) To: mgandhi, linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman On 06/01/2017 02:11 PM, 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@redhat.com> > --- Ahm and what happens to the fcoe_wq then? -- Johannes Thumshirn Storage jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function 2017-06-01 12:23 [PATCH] Eliminate extra 'out_free' label from fcoe_init function Milan P. Gandhi 2017-06-01 13:07 ` Johannes Thumshirn @ 2017-06-01 15:02 ` Dan Carpenter 2017-06-02 3:47 ` Milan P. Gandhi 1 sibling, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2017-06-01 15:02 UTC (permalink / raw) To: Milan P. Gandhi Cc: linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman 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@redhat.com> > --- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function 2017-06-01 15:02 ` Dan Carpenter @ 2017-06-02 3:47 ` Milan P. Gandhi 2017-06-02 5:49 ` Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Milan P. Gandhi @ 2017-06-02 3:47 UTC (permalink / raw) To: Dan Carpenter Cc: linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman 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@redhat.com> >> --- >> 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function 2017-06-02 3:47 ` Milan P. Gandhi @ 2017-06-02 5:49 ` Julia Lawall 2017-06-02 12:45 ` Milan P. Gandhi 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2017-06-02 5:49 UTC (permalink / raw) To: Milan P. Gandhi Cc: Dan Carpenter, linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman 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@redhat.com> > >> --- > >> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function 2017-06-02 5:49 ` Julia Lawall @ 2017-06-02 12:45 ` Milan P. Gandhi 0 siblings, 0 replies; 6+ messages in thread From: Milan P. Gandhi @ 2017-06-02 12:45 UTC (permalink / raw) To: Julia Lawall Cc: Dan Carpenter, linux-scsi, kernel-janitors, Johannes Thumshirn, Laurence Oberman On 06/02/2017 11:19 AM, Julia Lawall wrote: > > > 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@redhat.com> >>>> --- >>>> 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 > Hello Julia, Thanks for your hint! I have found a better way to remove a need for duplicate mutex_unlock statement and extra 'out_free' label. Will send the corrected path for the same. Many thanks, Milan. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-02 12:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-01 12:23 [PATCH] Eliminate extra 'out_free' label from fcoe_init function Milan P. Gandhi 2017-06-01 13:07 ` Johannes Thumshirn 2017-06-01 15:02 ` Dan Carpenter 2017-06-02 3:47 ` Milan P. Gandhi 2017-06-02 5:49 ` Julia Lawall 2017-06-02 12:45 ` Milan P. Gandhi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox