Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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
> 

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.
--
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



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux