| Message ID | 20230227030913.893004-2-void0red@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] hwmon: g762: add a check of devm_add_action in g762_of_clock_enable | expand |
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)
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); + if (ret) + return ret; ret = hid_parse(hdev); if (ret)