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