| Message ID | 20230524160352.19704-1-osmtendev@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | debugfs.c: Fix error checking for debugfs_create_dir | expand |
Osama Muhammad <osmtendev@gmail.com> writes: > This patch fixes the error checking in debugfs.c in > debugfs_create_dir. The correct way to check if an error occurred > is using 'IS_ERR' inline function. > > Signed-off-by: Osama Muhammad <osmtendev@gmail.com> The title is wrong, please see the wiki page below how to create titles. Also no need to say "This patch fixes..", saying "Fix..." is enough.
Hi, I will keep that in mind and send with the right subject while submitting a revision of the patch. Regarding the patch after researching more into it I have come to know that the debugfs API will not return null on error but an ERR_PTR. The modern wisdom about it is to ignore the errors returned by the function as stated in the comment above the function debugfs_create_file. > * NOTE: it's expected that most callers should _ignore_ the errors returned >* by this function. Other debugfs functions handle the fact that the "dentry" >* passed to them could be an error and they don't crash in that case. > * Drivers should generally work fine even if debugfs fails to init anyway. Here is the link to comment :- https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L451 Considering this, I will send the revision of the patch by removing error checks. Please correct me if there are any concerns with this. Thanks, Osmten On Tue, 30 May 2023 at 15:29, Kalle Valo <kvalo@kernel.org> wrote: > > Osama Muhammad <osmtendev@gmail.com> writes: > > > This patch fixes the error checking in debugfs.c in > > debugfs_create_dir. The correct way to check if an error occurred > > is using 'IS_ERR' inline function. > > > > Signed-off-by: Osama Muhammad <osmtendev@gmail.com> > > The title is wrong, please see the wiki page below how to create titles. > > Also no need to say "This patch fixes..", saying "Fix..." is enough. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi, In the previous email mistakenly I have referenced debugfs_create_file but it's the same for debugfs_create_dir also. Here is the link to comment above the function debugfs_create_dir https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L564 Thanks, Osmten On Tue, 30 May 2023 at 16:02, Osama Muhammad <osmtendev@gmail.com> wrote: > > Hi, > > I will keep that in mind and send with the right subject while > submitting a revision of the patch. > > Regarding the patch after researching more into it I have come to know > that the debugfs > API will not return null on error but an ERR_PTR. The modern wisdom > about it is to ignore the errors returned by the function as stated in > the comment above the function debugfs_create_file. > > > * NOTE: it's expected that most callers should _ignore_ the errors returned > >* by this function. Other debugfs functions handle the fact that the "dentry" > >* passed to them could be an error and they don't crash in that case. > > * Drivers should generally work fine even if debugfs fails to init anyway. > Here is the link to comment :- > https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L451 > > Considering this, I will send the revision of the patch by removing > error checks. Please correct me if there are any concerns with this. > > Thanks, > Osmten > > On Tue, 30 May 2023 at 15:29, Kalle Valo <kvalo@kernel.org> wrote: > > > > Osama Muhammad <osmtendev@gmail.com> writes: > > > > > This patch fixes the error checking in debugfs.c in > > > debugfs_create_dir. The correct way to check if an error occurred > > > is using 'IS_ERR' inline function. > > > > > > Signed-off-by: Osama Muhammad <osmtendev@gmail.com> > > > > The title is wrong, please see the wiki page below how to create titles. > > > > Also no need to say "This patch fixes..", saying "Fix..." is enough. > > > > -- > > https://patchwork.kernel.org/project/linux-wireless/list/ > > > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/mediatek/mt76/debugfs.c b/drivers/net/wireless/mediatek/mt76/debugfs.c index 57fbcc83e074..d9ba700131fd 100644 --- a/drivers/net/wireless/mediatek/mt76/debugfs.c +++ b/drivers/net/wireless/mediatek/mt76/debugfs.c @@ -109,7 +109,7 @@ mt76_register_debugfs_fops(struct mt76_phy *phy, struct dentry *dir; dir = debugfs_create_dir("mt76", phy->hw->wiphy->debugfsdir); - if (!dir) + if (IS_ERR(dir)) return NULL; debugfs_create_u8("led_pin", 0600, dir, &phy->leds.pin);
This patch fixes the error checking in debugfs.c in debugfs_create_dir. The correct way to check if an error occurred is using 'IS_ERR' inline function. Signed-off-by: Osama Muhammad <osmtendev@gmail.com> --- drivers/net/wireless/mediatek/mt76/debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)