[Drbd-dev] [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling

[Immad Mir]

Ah, Thanks for the feedback, Christopher.

Immad.
________________________________
From: Christoph Böhmwalder <christoph.boehmwalder at linbit.com>
Sent: Monday, May 15, 2023 2:26 PM
To: mirimmad at outlook.com <mirimmad at outlook.com>
Cc: axboe at kernel.dk <axboe at kernel.dk>; drbd-dev at lists.linbit.com <drbd-dev at lists.linbit.com>; lars.ellenberg at linbit.com <lars.ellenberg at linbit.com>; linux-block at vger.kernel.org <linux-block at vger.kernel.org>; linux-kernel at vger.kernel.org <linux-kernel at vger.kernel.org>; mirimmad17 at gmail.com <mirimmad17 at gmail.com>; philipp.reisner at linbit.com <philipp.reisner at linbit.com>; skhan at linuxfoundation.org <skhan at linuxfoundation.org>
Subject: Re: [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling

Am 14.05.23 um 16:10 schrieb mirimmad at outlook.com:
> From: Immad Mir <mirimmad17 at gmail.com>
>
> debugfs_create_dir and debugfs_create_symlink return ERR_PTR incase of
> a failure which must be checked with the inline function IS_ERR. This
> patch attempts to do the same.
>
> Signed-off-by: Immad Mir <mirimmad17 at gmail.com>
> ---
> Changes in v2:
>     - Fix indentation
>     - Fix potential memory leak
>
>  drivers/block/drbd/drbd_debugfs.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
> index 12460b584..ab431a3f2 100644
> --- a/drivers/block/drbd/drbd_debugfs.c
> +++ b/drivers/block/drbd/drbd_debugfs.c
> @@ -781,6 +781,7 @@ void drbd_debugfs_device_add(struct drbd_device *device)
>
>        snprintf(vnr_buf, sizeof(vnr_buf), "%u", device->vnr);
>        dentry = debugfs_create_dir(vnr_buf, vols_dir);
> +    if (IS_ERR(dentry)) goto fail;
>        device->debugfs_vol = dentry;
>
>        snprintf(minor_buf, sizeof(minor_buf), "%u", device->minor);
> @@ -789,9 +790,15 @@ void drbd_debugfs_device_add(struct drbd_device *device)
>        if (!slink_name)
>                goto fail;
>        dentry = debugfs_create_symlink(minor_buf, drbd_debugfs_minors, slink_name);
> -     device->debugfs_minor = dentry;
> -     kfree(slink_name);
> -     slink_name = NULL;
> +    if (!IS_ERR(dentry)) {
> +        device->debugfs_minor = dentry;
> +        kfree(slink_name);
> +        slink_name = NULL;
> +    } else {
> +        kfree(slink_name);
> +        slink_name = NULL;
> +        goto fail;
> +    }
>
>  #define DCF(name)    do {                                    \
>        dentry = debugfs_create_file(#name, 0440,       \
> --
> 2.40.0
>
>

Hi, thanks for the patch.

Please see this commit:

commit d27e84a305980ac61df0a6841059d0eb09b8283d
Author: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
Date:   Tue Jun 18 17:45:49 2019 +0200

    block: drbd: no need to check return value of debugfs_create functions

    When calling debugfs functions, there is no need to ever check the
    return value.  The function can work or not, but the code logic should
    never do something different based on this.

Also, it still looks like that whitespace is garbled.

NAK.

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linbit.com/pipermail/drbd-dev/attachments/20230515/e76de2fd/attachment.htm>