Message ID | 20230416083028.14044-1-ysxu@hust.edu.cn (mailing list archive) |
---|---|
State | Rejected |
Delegated to: | Johannes Berg |
Headers | show |
Series | net: mac80211: use IS_ERR to check return value | expand |
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
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