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 <[email protected]>
---
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);
--
Em Mon, 13 Dec 2021 15:48:33 +0800
Xiaoke Wang <[email protected]> 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 <[email protected]>
> ---
> 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
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.