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

[walter harms <wharms@bfs.de>]

Am 02.06.2017 18:07, schrieb Julia Lawall:
> 
> 
> On Fri, 2 Jun 2017, walter harms wrote:
> 
>>
>>
>> Am 02.06.2017 14:39, schrieb Milan P. Gandhi:
>>> 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 and duplicate mutex_unlock statement.
>>>
>>> Signed-off-by: Milan P. Gandhi <mgandhi@redhat.com>
>>> ---
>>>  drivers/scsi/fcoe/fcoe.c | 7 +++----
>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>> index ea21e7b..a2cf3d0 100644
>>> --- a/drivers/scsi/fcoe/fcoe.c
>>> +++ b/drivers/scsi/fcoe/fcoe.c
>>> @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void)
>>>  	fcoe_dev_setup();
>>>
>>>  	rc = fcoe_if_init();
>>> +	mutex_unlock(&fcoe_config_mutex);
>>> +
>>>  	if (rc)
>>> -		goto out_free;
>>> +		goto out_destroy;
>>>
>>> -	mutex_unlock(&fcoe_config_mutex);
>>>  	return 0;
>>>
>> if you do that, why not
>> if (!rc) return 0;
> 
> I agree with Dan.  If's should be for failures.
> 

yep, but it "feels" also wrong to skip of a simple return.
I do not see a good solution.

re,
 wh

> julia
> 
>>
>> re,
>>  wh
>>
>>
>>
>>> -out_free:
>>> -	mutex_unlock(&fcoe_config_mutex);
>>>  out_destroy:
>>>  	destroy_workqueue(fcoe_wq);
>>>  	return rc;
>>> --
>>> 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
>>>
>> --
>> 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
>>
>