Re: [PATCH] platform/x86: system76: Reducing redundant conditional judgments in system76_add()

[Ilpo Järvinen]

On Tue, 20 Aug 2024, Xi Huang wrote:

> In case of an error, goto jumps to the “error” label,
> where the if (data->has_open_ec) check is redundant in most cases.
> Since the conditions for most goto statements have already
> been satisfied by if (data->has_open_ec),the code has been modified to
> improve execution speed.

And why would the error rollback path need to be improved in execution 
speed? In any case, this change is going to below the noise level when it 
comes to a measurable impact.

I'm sorry but I don't think this change is useful.

-- 
 i.

> Signed-off-by: Xi Huang <xuiagnh@xxxxxxxxx>
> ---
>  drivers/platform/x86/system76_acpi.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
> index 3da753b3d..05b4bf18f 100644
> --- a/drivers/platform/x86/system76_acpi.c
> +++ b/drivers/platform/x86/system76_acpi.c
> @@ -757,33 +757,34 @@ static int system76_add(struct acpi_device *acpi_dev)
>  
>  	err = input_register_device(data->input);
>  	if (err)
> -		goto error;
> +		if (data->has_open_ec)
> +			goto free_error;
> +		else
> +			return err;
>  
>  	if (data->has_open_ec) {
>  		err = system76_get_object(data, "NFAN", &data->nfan);
>  		if (err)
> -			goto error;
> +			goto free_error;
>  
>  		err = system76_get_object(data, "NTMP", &data->ntmp);
>  		if (err)
> -			goto error;
> +			goto free_error;
>  
>  		data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
>  			"system76_acpi", data, &thermal_chip_info, NULL);
>  		err = PTR_ERR_OR_ZERO(data->therm);
>  		if (err)
> -			goto error;
> +			goto free_error;
>  
>  		system76_battery_init();
>  	}
>  
>  	return 0;
>  
> -error:
> -	if (data->has_open_ec) {
> -		kfree(data->ntmp);
> -		kfree(data->nfan);
> -	}
> +free_error:
> +	kfree(data->ntmp);
> +	kfree(data->nfan);
>  	return err;
>  }
>  
>