Re: [PATCH 1/1] debugfs: use IS_ERR to check for error

From: Christian König
Date: Fri Sep 03 2021 - 02:27:43 EST

Am 02.09.21 um 19:01 schrieb Das, Nirmoy:

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.

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;

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 just realized that we don't return NULL on error anymore:

commit ff9fb72bc07705c00795ca48631f7fffe24d2c6b
Author: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Date:   Wed Jan 23 11:28:14 2019 +0100

    debugfs: return error values, not NULL

and the current doc also says "If an error occurs, ERR_PTR(-ERROR) will be returned."

If I am not missing anything, this patch should be fine.

Ah! Yes, now that makes sense.

Looks like that my memory and the documentation under is outdated.

I can update my memory, but I have no idea where this documentation comes from and how to fix it.




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.


greg k-h