| Message ID | 2db68591-64e2-4f43-a5e1-cb8849f0a296@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ASoC: soc-card: soc-card-test: Fix some error handling in probe() | expand |
On 10/4/24 15:22, Dan Carpenter wrote: > Fix this reversed if statement and call put_device() before returning > the error code. > > Fixes: ef7784e41db7 ("ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > v2: call put_device() > > sound/soc/soc-card-test.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/soc-card-test.c b/sound/soc/soc-card-test.c > index 075c52fe82e5..faf9a3d46884 100644 > --- a/sound/soc/soc-card-test.c > +++ b/sound/soc/soc-card-test.c > @@ -148,8 +148,10 @@ static int soc_card_test_case_init(struct kunit *test) > priv->card->owner = THIS_MODULE; > > ret = snd_soc_register_card(priv->card); > - if (!ret) > + if (ret) { > + put_device(priv->card_dev); > return ret; > + } > > return 0; > } Thanks. Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com> I can see that put_device() is also missing earlier in the function: if (!priv->card) return -ENOMEM; I can send a fix for that.
On Wed, Apr 10, 2024 at 03:43:45PM +0100, Richard Fitzgerald wrote: > On 10/4/24 15:22, Dan Carpenter wrote: > > Fix this reversed if statement and call put_device() before returning > > the error code. > > > > Fixes: ef7784e41db7 ("ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > --- > > v2: call put_device() > > > > sound/soc/soc-card-test.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/sound/soc/soc-card-test.c b/sound/soc/soc-card-test.c > > index 075c52fe82e5..faf9a3d46884 100644 > > --- a/sound/soc/soc-card-test.c > > +++ b/sound/soc/soc-card-test.c > > @@ -148,8 +148,10 @@ static int soc_card_test_case_init(struct kunit *test) > > priv->card->owner = THIS_MODULE; > > ret = snd_soc_register_card(priv->card); > > - if (!ret) > > + if (ret) { > > + put_device(priv->card_dev); > > return ret; > > + } > > return 0; > > } > > Thanks. > Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com> > > I can see that put_device() is also missing earlier in the > function: > > if (!priv->card) > return -ENOMEM; > > I can send a fix for that. No. Let me resend. I'm sorry, this patch has not been up to proper standards. Also I should fix Smatch to warn about missing put_device() calls to prevent this sort of thing going forward. regards, dan carpenter
On 10/4/24 16:31, Dan Carpenter wrote: > On Wed, Apr 10, 2024 at 03:43:45PM +0100, Richard Fitzgerald wrote: >> On 10/4/24 15:22, Dan Carpenter wrote: >>> Fix this reversed if statement and call put_device() before returning >>> the error code. >>> >>> Fixes: ef7784e41db7 ("ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol") >>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> >>> --- >>> v2: call put_device() >>> >>> sound/soc/soc-card-test.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/sound/soc/soc-card-test.c b/sound/soc/soc-card-test.c >>> index 075c52fe82e5..faf9a3d46884 100644 >>> --- a/sound/soc/soc-card-test.c >>> +++ b/sound/soc/soc-card-test.c >>> @@ -148,8 +148,10 @@ static int soc_card_test_case_init(struct kunit *test) >>> priv->card->owner = THIS_MODULE; >>> ret = snd_soc_register_card(priv->card); >>> - if (!ret) >>> + if (ret) { >>> + put_device(priv->card_dev); >>> return ret; >>> + } >>> return 0; >>> } >> >> Thanks. >> Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com> >> >> I can see that put_device() is also missing earlier in the >> function: >> >> if (!priv->card) >> return -ENOMEM; >> >> I can send a fix for that. > > No. Let me resend. I'm sorry, this patch has not been up to proper > standards. The same could be said for my original code here. I suggest moving this block of code _before_ the kunit_device_register() so there's no need to put_device() if the alloc fails: priv->card = kunit_kzalloc(test, sizeof(*priv->card), GFP_KERNEL); if (!priv->card) return -ENOMEM; Also I should fix Smatch to warn about missing put_device() > calls to prevent this sort of thing going forward. > > regards, > dan carpenter
diff --git a/sound/soc/soc-card-test.c b/sound/soc/soc-card-test.c index 075c52fe82e5..faf9a3d46884 100644 --- a/sound/soc/soc-card-test.c +++ b/sound/soc/soc-card-test.c @@ -148,8 +148,10 @@ static int soc_card_test_case_init(struct kunit *test) priv->card->owner = THIS_MODULE; ret = snd_soc_register_card(priv->card); - if (!ret) + if (ret) { + put_device(priv->card_dev); return ret; + } return 0; }
Fix this reversed if statement and call put_device() before returning the error code. Fixes: ef7784e41db7 ("ASoC: soc-card: Add KUnit test case for snd_soc_card_get_kcontrol") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- v2: call put_device() sound/soc/soc-card-test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)