[PATCH platform-next 1/1] platform/mellanox: mlxreg-lc: Make error handling flow consistent

[PATCH platform-next 1/1] platform/mellanox: mlxreg-lc: Make error handling flow consistent

[Vadim Pasternak]

Use 'goto' statement in error flow of mlxreg_lc_event_handler() at all
places for consistency.

This follow-up patch implementing comments from
https://www.spinics.net/lists/platform-driver-x86/msg34587.html

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
 drivers/platform/mellanox/mlxreg-lc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c
index 1e071df4c9f5..d47fe22080e6 100644
--- a/drivers/platform/mellanox/mlxreg-lc.c
+++ b/drivers/platform/mellanox/mlxreg-lc.c
@@ -564,10 +564,8 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
 		 mlxreg_lc->data->slot, mlxreg_lc->state, kind, action);
 
 	mutex_lock(&mlxreg_lc->lock);
-	if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) {
-		mutex_unlock(&mlxreg_lc->lock);
-		return 0;
-	}
+	if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED))
+		goto mlxreg_lc_non_initialzed_exit;
 
 	switch (kind) {
 	case MLXREG_HOTPLUG_LC_SYNCED:
@@ -594,8 +592,7 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
 				/* In case line card is configured - enable it. */
 				if (mlxreg_lc->state & MLXREG_LC_CONFIGURED)
 					err = mlxreg_lc_enable_disable(mlxreg_lc, 1);
-				mutex_unlock(&mlxreg_lc->lock);
-				return err;
+					goto mlxreg_lc_enable_disable_exit;
 			}
 			err = mlxreg_lc_create_static_devices(mlxreg_lc, mlxreg_lc->main_devs,
 							      mlxreg_lc->main_devs_num);
@@ -627,8 +624,10 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
 		break;
 	}
 
+mlxreg_lc_enable_disable_exit:
 mlxreg_lc_power_on_off_fail:
 mlxreg_lc_create_static_devices_fail:
+mlxreg_lc_non_initialzed_exit:
 	mutex_unlock(&mlxreg_lc->lock);
 
 	return err;
-- 
2.20.1

Re: [PATCH platform-next 1/1] platform/mellanox: mlxreg-lc: Make error handling flow consistent

[Hans de Goede]

Hi,

On 9/4/22 16:11, Vadim Pasternak wrote:
> Use 'goto' statement in error flow of mlxreg_lc_event_handler() at all
> places for consistency.
> 
> This follow-up patch implementing comments from
> https://www.spinics.net/lists/platform-driver-x86/msg34587.html
> 
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> ---
>  drivers/platform/mellanox/mlxreg-lc.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c
> index 1e071df4c9f5..d47fe22080e6 100644
> --- a/drivers/platform/mellanox/mlxreg-lc.c
> +++ b/drivers/platform/mellanox/mlxreg-lc.c
> @@ -564,10 +564,8 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
>  		 mlxreg_lc->data->slot, mlxreg_lc->state, kind, action);
>  
>  	mutex_lock(&mlxreg_lc->lock);
> -	if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) {
> -		mutex_unlock(&mlxreg_lc->lock);
> -		return 0;
> -	}
> +	if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED))
> +		goto mlxreg_lc_non_initialzed_exit;
>  
>  	switch (kind) {
>  	case MLXREG_HOTPLUG_LC_SYNCED:
> @@ -594,8 +592,7 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
>  				/* In case line card is configured - enable it. */
>  				if (mlxreg_lc->state & MLXREG_LC_CONFIGURED)
>  					err = mlxreg_lc_enable_disable(mlxreg_lc, 1);
> -				mutex_unlock(&mlxreg_lc->lock);
> -				return err;
> +					goto mlxreg_lc_enable_disable_exit;

The intend of this goto is wrong (one indent level too much).

I have merged this now with the indent fixed up:

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


>  			}
>  			err = mlxreg_lc_create_static_devices(mlxreg_lc, mlxreg_lc->main_devs,
>  							      mlxreg_lc->main_devs_num);
> @@ -627,8 +624,10 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind,
>  		break;
>  	}
>  
> +mlxreg_lc_enable_disable_exit:
>  mlxreg_lc_power_on_off_fail:
>  mlxreg_lc_create_static_devices_fail:
> +mlxreg_lc_non_initialzed_exit:
>  	mutex_unlock(&mlxreg_lc->lock);
>  
>  	return err;