<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
Ah, Thanks for the feedback, Christopher.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
Immad.</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Christoph Böhmwalder <christoph.boehmwalder@linbit.com><br>
<b>Sent:</b> Monday, May 15, 2023 2:26 PM<br>
<b>To:</b> mirimmad@outlook.com <mirimmad@outlook.com><br>
<b>Cc:</b> axboe@kernel.dk <axboe@kernel.dk>; drbd-dev@lists.linbit.com <drbd-dev@lists.linbit.com>; lars.ellenberg@linbit.com <lars.ellenberg@linbit.com>; linux-block@vger.kernel.org <linux-block@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>;
mirimmad17@gmail.com <mirimmad17@gmail.com>; philipp.reisner@linbit.com <philipp.reisner@linbit.com>; skhan@linuxfoundation.org <skhan@linuxfoundation.org><br>
<b>Subject:</b> Re: [PATCH v2] drdb: fix debugfs_create_dir and debugfs_create_symlink error handling</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Am 14.05.23 um 16:10 schrieb mirimmad@outlook.com:<br>
> From: Immad Mir <mirimmad17@gmail.com><br>
> <br>
> debugfs_create_dir and debugfs_create_symlink return ERR_PTR incase of<br>
> a failure which must be checked with the inline function IS_ERR. This<br>
> patch attempts to do the same.<br>
> <br>
> Signed-off-by: Immad Mir <mirimmad17@gmail.com><br>
> ---<br>
> Changes in v2:<br>
> - Fix indentation<br>
> - Fix potential memory leak<br>
> <br>
> drivers/block/drbd/drbd_debugfs.c | 13 ++++++++++---<br>
> 1 file changed, 10 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c<br>
> index 12460b584..ab431a3f2 100644<br>
> --- a/drivers/block/drbd/drbd_debugfs.c<br>
> +++ b/drivers/block/drbd/drbd_debugfs.c<br>
> @@ -781,6 +781,7 @@ void drbd_debugfs_device_add(struct drbd_device *device)<br>
> <br>
> snprintf(vnr_buf, sizeof(vnr_buf), "%u", device->vnr);<br>
> dentry = debugfs_create_dir(vnr_buf, vols_dir);<br>
> + if (IS_ERR(dentry)) goto fail;<br>
> device->debugfs_vol = dentry;<br>
> <br>
> snprintf(minor_buf, sizeof(minor_buf), "%u", device->minor);<br>
> @@ -789,9 +790,15 @@ void drbd_debugfs_device_add(struct drbd_device *device)<br>
> if (!slink_name)<br>
> goto fail;<br>
> dentry = debugfs_create_symlink(minor_buf, drbd_debugfs_minors, slink_name);<br>
> - device->debugfs_minor = dentry;<br>
> - kfree(slink_name);<br>
> - slink_name = NULL;<br>
> + if (!IS_ERR(dentry)) {<br>
> + device->debugfs_minor = dentry;<br>
> + kfree(slink_name);<br>
> + slink_name = NULL;<br>
> + } else {<br>
> + kfree(slink_name);<br>
> + slink_name = NULL;<br>
> + goto fail;<br>
> + }<br>
> <br>
> #define DCF(name) do { \<br>
> dentry = debugfs_create_file(#name, 0440, \<br>
> --<br>
> 2.40.0<br>
> <br>
> <br>
<br>
Hi, thanks for the patch.<br>
<br>
Please see this commit:<br>
<br>
commit d27e84a305980ac61df0a6841059d0eb09b8283d<br>
Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org><br>
Date: Tue Jun 18 17:45:49 2019 +0200<br>
<br>
block: drbd: no need to check return value of debugfs_create functions<br>
<br>
When calling debugfs functions, there is no need to ever check the<br>
return value. The function can work or not, but the code logic should<br>
never do something different based on this.<br>
<br>
Also, it still looks like that whitespace is garbled.<br>
<br>
NAK.<br>
<br>
-- <br>
Christoph Böhmwalder<br>
LINBIT | Keeping the Digital World Running<br>
DRBD HA — Disaster Recovery — Software defined Storage<br>
</div>
</span></font></div>
</body>
</html>