On Saturday 24 August 2024 22:30:57 Dmitry Torokhov wrote:
> This makes the code more compact and error handling more robust
> by ensuring that mutexes are released in all code paths when control
> leaves critical section.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
> drivers/input/mouse/alps.c | 48 +++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index d5ef5a112d6f..4e37fc3f1a9e 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1396,24 +1396,16 @@ static bool alps_is_valid_package_ss4_v2(struct psmouse *psmouse)
>
> static DEFINE_MUTEX(alps_mutex);
>
> -static void alps_register_bare_ps2_mouse(struct work_struct *work)
> +static int alps_do_register_bare_ps2_mouse(struct alps_data *priv)
> {
> - struct alps_data *priv =
> - container_of(work, struct alps_data, dev3_register_work.work);
> struct psmouse *psmouse = priv->psmouse;
> struct input_dev *dev3;
> - int error = 0;
> -
> - mutex_lock(&alps_mutex);
> -
> - if (priv->dev3)
> - goto out;
> + int error;
>
> dev3 = input_allocate_device();
> if (!dev3) {
> psmouse_err(psmouse, "failed to allocate secondary device\n");
> - error = -ENOMEM;
> - goto out;
> + return -ENOMEM;
> }
>
> snprintf(priv->phys3, sizeof(priv->phys3), "%s/%s",
> @@ -1446,21 +1438,35 @@ static void alps_register_bare_ps2_mouse(struct work_struct *work)
> psmouse_err(psmouse,
> "failed to register secondary device: %d\n",
> error);
> - input_free_device(dev3);
> - goto out;
> + goto err_free_input;
> }
>
> priv->dev3 = dev3;
> + return 0;
>
> -out:
> - /*
> - * Save the error code so that we can detect that we
> - * already tried to create the device.
> - */
> - if (error)
> - priv->dev3 = ERR_PTR(error);
> +err_free_input:
> + input_free_device(dev3);
> + return error;
> +}
>
> - mutex_unlock(&alps_mutex);
> +static void alps_register_bare_ps2_mouse(struct work_struct *work)
> +{
> + struct alps_data *priv = container_of(work, struct alps_data,
> + dev3_register_work.work);
> + int error;
> +
> + guard(mutex)(&alps_mutex);
> +
> + if (!priv->dev3) {
> + error = alps_do_register_bare_ps2_mouse(priv);
> + if (error) {
> + /*
> + * Save the error code so that we can detect that we
> + * already tried to create the device.
> + */
> + priv->dev3 = ERR_PTR(error);
> + }
> + }
> }
>
> static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> --
> 2.46.0.295.g3b9ea8a38a-goog
>
>
> --
> Dmitry
Hello, I'm not familiar with new guards and their usage. But if this is
a preferred way for handling mutexes then go ahead.
I just looked at the code and I do not see any obvious error neither in
old nor in new version.
