* [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup()
@ 2021-12-13 7:48 Xiaoke Wang
2021-12-14 15:03 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 4+ messages in thread
From: Xiaoke Wang @ 2021-12-13 7:48 UTC (permalink / raw)
To: crope, mchehab; +Cc: linux-media, linux-kernel, Xiaoke Wang
Note: Compare with the last email, this one is using my full name.
kstrdup() returns NULL if some internal memory errors happen, it is
better to check the return value of it. Since the return type of
dvb_usbv2_disconnect() is void, so only raise the error info.
Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com>
---
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index f1c79f3..a43a310 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -1009,6 +1009,9 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
const char *devname = kstrdup(dev_name(&d->udev->dev), GFP_KERNEL);
const char *drvname = d->name;
+ if (!devname)
+ dev_err(&d->udev->dev, "%s: kstrdup() failed\n", KBUILD_MODNAME);
+
dev_dbg(&d->udev->dev, "%s: bInterfaceNumber=%d\n", __func__,
intf->cur_altsetting->desc.bInterfaceNumber);
@@ -1023,9 +1026,14 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
kfree(d->priv);
kfree(d);
- pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
- KBUILD_MODNAME, drvname, devname);
- kfree(devname);
+ if (devname) {
+ pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
+ KBUILD_MODNAME, drvname, devname);
+ kfree(devname);
+ } else {
+ pr_info("%s: '%s:UNKNOWN' successfully deinitialized and disconnected\n",
+ KBUILD_MODNAME, drvname);
+ }
}
EXPORT_SYMBOL(dvb_usbv2_disconnect);
--
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup()
2021-12-13 7:48 [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup() Xiaoke Wang
@ 2021-12-14 15:03 ` Mauro Carvalho Chehab
2021-12-14 15:51 ` Xiaoke Wang
0 siblings, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-12-14 15:03 UTC (permalink / raw)
To: Xiaoke Wang; +Cc: crope, linux-media, linux-kernel
Em Mon, 13 Dec 2021 15:48:33 +0800
Xiaoke Wang <xkernel.wang@foxmail.com> escreveu:
> Note: Compare with the last email, this one is using my full name.
> kstrdup() returns NULL if some internal memory errors happen, it is
> better to check the return value of it. Since the return type of
> dvb_usbv2_disconnect() is void, so only raise the error info.
>
> Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com>
> ---
> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> index f1c79f3..a43a310 100644
> --- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> +++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> @@ -1009,6 +1009,9 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
> const char *devname = kstrdup(dev_name(&d->udev->dev), GFP_KERNEL);
> const char *drvname = d->name;
>
> + if (!devname)
> + dev_err(&d->udev->dev, "%s: kstrdup() failed\n", KBUILD_MODNAME);
> +
Don't use KBUILD_MODNAME, as dev_err will already add the driver/device's
name.
> dev_dbg(&d->udev->dev, "%s: bInterfaceNumber=%d\n", __func__,
> intf->cur_altsetting->desc.bInterfaceNumber);
>
> @@ -1023,9 +1026,14 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
> kfree(d->priv);
> kfree(d);
>
> - pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
> - KBUILD_MODNAME, drvname, devname);
Better to use:
dev_dbg(&d->udev->dev, "successfully deinitialized and disconnected\n");
> - kfree(devname);
No need to place kfree() inside an if, as kfree(NULL) is safe.
> + if (devname) {
> + pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
> + KBUILD_MODNAME, drvname, devname);
> + kfree(devname);
> + } else {
> + pr_info("%s: '%s:UNKNOWN' successfully deinitialized and disconnected\n",
> + KBUILD_MODNAME, drvname);
> + }
> }
> EXPORT_SYMBOL(dvb_usbv2_disconnect);
>
Thanks,
Mauro
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup()
2021-12-14 15:03 ` Mauro Carvalho Chehab
@ 2021-12-14 15:51 ` Xiaoke Wang
0 siblings, 0 replies; 4+ messages in thread
From: Xiaoke Wang @ 2021-12-14 15:51 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: crope, linux-media, linux-kernel
On Tue, Dec 14, 2021 11:03 PM +0800, Mauro Carvalho Chehab wrote:
>> + if (!devname)
>> + dev_err(&d->udev->dev, "%s: kstrdup() failed\n", KBUILD_MODNAME);
>> +
> Don't use KBUILD_MODNAME, as dev_err will already add the driver/device's
> name.
Here I know I need to change it to: dev_err(&d->udev->dev, "kstrdup() failed\n");
>> - pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
>> - KBUILD_MODNAME, drvname, devname);
>
> Better to use:
> dev_dbg(&d->udev->dev, "successfully deinitialized and disconnected\n");
>
>> - kfree(devname);
> No need to place kfree() inside an if, as kfree(NULL) is safe.
Here I know I need to put kfree() at the end of this function.
However, if I replace the original pr_info(...) with:
> dev_dbg(&d->udev->dev, "successfully deinitialized and disconnected\n");
Then devname and drvname seems can be all removed, as they are useless
in dvb_usbv2_disconnect() anymore. I am not sure if I understand correctly.
If so, I will just remove them and change pr_info() -> dev_dbg() in next
version.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup()
@ 2021-12-12 14:52 xkernel
0 siblings, 0 replies; 4+ messages in thread
From: xkernel @ 2021-12-12 14:52 UTC (permalink / raw)
To: crope, mchehab; +Cc: linux-media, linux-kernel, xkernel
kstrdup() returns NULL if some internal memory errors happen, it is
better to check the return value of it. Since the return type of
dvb_usbv2_disconnect() is void, so only raise the error info.
Signed-off-by: xkernel <xkernel.wang@foxmail.com>
---
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index f1c79f3..a43a310 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -1009,6 +1009,9 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
const char *devname = kstrdup(dev_name(&d->udev->dev), GFP_KERNEL);
const char *drvname = d->name;
+ if (!devname)
+ dev_err(&d->udev->dev, "%s: kstrdup() failed\n", KBUILD_MODNAME);
+
dev_dbg(&d->udev->dev, "%s: bInterfaceNumber=%d\n", __func__,
intf->cur_altsetting->desc.bInterfaceNumber);
@@ -1023,9 +1026,14 @@ void dvb_usbv2_disconnect(struct usb_interface *intf)
kfree(d->priv);
kfree(d);
- pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
- KBUILD_MODNAME, drvname, devname);
- kfree(devname);
+ if (devname) {
+ pr_info("%s: '%s:%s' successfully deinitialized and disconnected\n",
+ KBUILD_MODNAME, drvname, devname);
+ kfree(devname);
+ } else {
+ pr_info("%s: '%s:UNKNOWN' successfully deinitialized and disconnected\n",
+ KBUILD_MODNAME, drvname);
+ }
}
EXPORT_SYMBOL(dvb_usbv2_disconnect);
--
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-14 15:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-13 7:48 [PATCH] media: usb: dvb-usb-v2: check the return value of kstrdup() Xiaoke Wang
2021-12-14 15:03 ` Mauro Carvalho Chehab
2021-12-14 15:51 ` Xiaoke Wang
-- strict thread matches above, loose matches on Subject: below --
2021-12-12 14:52 xkernel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.