Re: [PATCH 1/1] debugfs: use IS_ERR to check for error
From: Das, Nirmoy
Date: Thu Sep 02 2021 - 12:48:43 EST
On 9/2/2021 6:34 PM, Greg KH wrote:
On Thu, Sep 02, 2021 at 05:10:24PM +0200, Christian König wrote:
Am 02.09.21 um 14:20 schrieb Greg KH:
On Thu, Sep 02, 2021 at 02:03:12PM +0200, Christian König wrote:
Am 02.09.21 um 12:38 schrieb Greg KH:
On Thu, Sep 02, 2021 at 12:29:17PM +0200, Nirmoy Das wrote:
debugfs_create_file() returns encoded error so
use IS_ERR for checking return value.
References: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1686&data=04%7C01%7Cnirmoy.das%40amd.com%7C7a1f1095c0d64416576c08d96e2f7b38%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637661973378236086%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=g9BQRJG8gvjGFq6oj5vk9PCemQ39U19CLmkMNHVUafg%3D&reserved=0
Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>
---
fs/debugfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 8129a430d789..2f117c57160d 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -528,7 +528,7 @@ void debugfs_create_file_size(const char *name, umode_t mode,
{
struct dentry *de = debugfs_create_file(name, mode, parent, data, fops);
- if (de)
+ if (!IS_ERR(de))
d_inode(de)->i_size = file_size;
}
EXPORT_SYMBOL_GPL(debugfs_create_file_size);
--
2.32.0
Ah, good catch, I'll queue this up after 5.15-rc1 is out, thanks!
Thinking more about this if I'm not completely mistaken
debugfs_create_file() returns -ENODEV when debugfs is disabled and NULL on
any other error.
How can this function be called if debugfs is not enabled in the system
configuration? This _is_ the debugfs core code.
Well, that's what I meant. The original code is correct and Nirmoy's patch
here is breaking it.
Ah, yes, sorry, you are right. This function can not return an error
value, if something went wrong, the result will always be NULL.
[I pressed send too early in my previous email :/ ]
The issue occurs when CONFIG_DEBUG_FS=y and CONFIG_DEBUG_FS_ALLOW_NONE=y
config options are set, so a call to
debugfs_create_file() will return ERR_PTR(-EPERM) instead of NULL. So I
think it still makes sense to keep the check with IS_ERR_OR_NULL()
Regards,
Nirmoy
Nirmoys other patch is for a driver and there the function can indeed return
both error code and NULL.
You should never be checking this stuff in a caller anyway, so no, don't
do it there either.
thanks,
greg k-h