Messages in this thread | | | Date | Sun, 26 Feb 2023 20:21:42 -0800 | From | Guenter Roeck <> | Subject | Re: [PATCH v2 2/2] hwmon: nzxt-smart2: add a check of devm_add_action in nzxt_smart2_hid_probe |
| |
On 2/26/23 19:09, void0red wrote: > From: Kang Chen <void0red@gmail.com> > > devm_add_action may fails, check it and return early. > > Signed-off-by: Kang Chen <void0red@gmail.com> > --- > v2 -> v1: split the patch > > drivers/hwmon/nzxt-smart2.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c > index 2b93ba896..725974cb3 100644 > --- a/drivers/hwmon/nzxt-smart2.c > +++ b/drivers/hwmon/nzxt-smart2.c > @@ -737,8 +737,10 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev, > init_waitqueue_head(&drvdata->wq); > > mutex_init(&drvdata->mutex); > - devm_add_action(&hdev->dev, (void (*)(void *))mutex_destroy, > + ret = devm_add_action(&hdev->dev, (void (*)(void *))mutex_destroy, > &drvdata->mutex);
Please watch for multi-line alignment. Also, turns out the original code is wrong: Type casting a function pointer argument like this, while it typically works in practice, is undefined in C. The function pointer passed to devm_add_action() needs to point to a local function with void * argument, and that function needs to call mutex_destroy() with the same argument. Also, based on the context, this needs to call devm_add_action_or_reset() to ensure that the destroy function is called on error.
Thanks, Guenter
> + if (ret) > + return ret; > > ret = hid_parse(hdev); > if (ret)
| |