According to the annotation of function debugfs_create_fs, if
an error occurs, ERR_PTR(-ERROR) will be returned instead of
a null pointer or zero value.

Fix it by using IS_ERR().

Fixes: e9f207f0ff90 ("[MAC80211]: Add debugfs attributes.")
Signed-off-by: yingsha xu <ysxu@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
---
 net/mac80211/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index dfb9f55e2685..672bf969ad88 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -674,7 +674,7 @@ void debugfs_hw_add(struct ieee80211_local *local)
 	statsd = debugfs_create_dir("statistics", phyd);
 
 	/* if the dir failed, don't put all the other things into the root! */
-	if (!statsd)
+	if (IS_ERR(statsd))
 		return;
 
 #ifdef CONFIG_MAC80211_DEBUG_COUNTERS
-- 
2.17.1

On Sun, 2023-04-16 at 16:30 +0800, yingsha xu wrote:
> According to the annotation of function debugfs_create_fs, if
> an error occurs, ERR_PTR(-ERROR) will be returned instead of
> a null pointer or zero value.
> 
> Fix it by using IS_ERR().

I don't this this is right, or fixed anything ...

If debugfs indeed returned an ERR_PTR() value, then the later debugfs
adds will do nothing.

Since it doesn't look like debugfs_create_dir() can actually return NULL
these days (not sure it ever could), I guess we can even remove the
check.

But you could've just read the comment there too, to know what the NULL
check was about ...

johannes

On Tue, Apr 18, 2023 at 10:36:14AM +0200, Johannes Berg wrote:
> On Sun, 2023-04-16 at 16:30 +0800, yingsha xu wrote:
> > According to the annotation of function debugfs_create_fs, if
> > an error occurs, ERR_PTR(-ERROR) will be returned instead of
> > a null pointer or zero value.
> > 
> > Fix it by using IS_ERR().
> 
> I don't this this is right, or fixed anything ...
> 
> If debugfs indeed returned an ERR_PTR() value, then the later debugfs
> adds will do nothing.
> 
> Since it doesn't look like debugfs_create_dir() can actually return NULL
> these days (not sure it ever could), I guess we can even remove the
> check.
> 

Correct.  They have a patch ready which deletes the check and the
comment.  Someone should have replied to this thread to NAK their own
patch so that you didn't bother reviewing it.

> But you could've just read the comment there too, to know what the NULL
> check was about ...

The comment was always wrong.  Debugfs could return NULL but then
the other debugfs functions turned into no ops...

regards,
dan carpenter