Messages in this thread | | | Subject | Re: [PATCH 1/1] debugfs: use IS_ERR to check for error | From | "Das, Nirmoy" <> | Date | Thu, 2 Sep 2021 18:48:34 +0200 |
| |
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@amd.com> >>>>>> --- >>>>>> 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
| |