2023-05-14 13:06:37

by Immad Mir

Subject: [PATCH] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling

From: Immad Mir <[email protected]>

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 <[email protected]>
---
drivers/block/drbd/drbd_debugfs.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
index 12460b584..1cec65563 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,12 @@ 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);
+ if (!IS_ERR(dentry)) {
device->debugfs_minor = dentry;
kfree(slink_name);
slink_name = NULL;
+ } else
+ goto fail;

#define DCF(name) do { \
dentry = debugfs_create_file(#name, 0440, \
--
2.40.0

2023-05-14 14:51:04

by Immad Mir

Subject: [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling

From: Immad Mir <[email protected]>

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 <[email protected]>
---
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

2023-05-15 09:05:51

by Christoph Böhmwalder

Subject: Re: [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling

Am 14.05.23 um 16:10 schrieb [email protected]:
> From: Immad Mir <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
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