Michal Kazior <michal.kazior@xxxxxxxxx> writes:
> This fixes failpath when override AC pdev param
> setup fails and makes other pdev params setting
> fail as well.
>
> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
[...]
> ar->num_started_vdevs = 0;
> ath10k_regd_update(ar);
> ret = 0;
> + goto out;
>
> -exit:
> +err_core_stop:
> + ath10k_core_stop(ar);
> +err_power_down:
> + ath10k_hif_power_down(ar);
> +err_off:
> + ar->state = ATH10K_STATE_OFF;
> +out:
> mutex_unlock(&ar->conf_mutex);
> return ret;
> }
Having err_ labels on the "main" code path is not good, the error
handling code should be clearly separated to make it easier to read. I
think you should use the same style as pci.c uses, for example something
like this:
mutex_unlock(&ar->conf_mutex);
return 0;
err_core_stop:
ath10k_core_stop(ar);
err_power_down:
ath10k_hif_power_down(ar);
err_off:
ar->state = ATH10K_STATE_OFF;
mutex_unlock(&ar->conf_mutex);
return ret;
I know this has mutex_unlock() so it's not good either, but I think it's
still better. Other option might be to do like this:
ret = 0;
out:
mutex_unlock(&ar->conf_mutex);
return ret;
err_core_stop:
ath10k_core_stop(ar);
err_power_down:
ath10k_hif_power_down(ar);
err_off:
ar->state = ATH10K_STATE_OFF;
goto out;
But not sure if it's any better. More ideas?
Oh yeah, please also add an empty line before each label.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
