[PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove

[Fabian Frederick]

Fix checkpatch warnings:

WARNING: debugfs_remove(NULL) is safe this check is probably not required

Signed-off-by: Fabian Frederick <***@skynet.be>
---
net/mac80211/debugfs_key.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/debugfs_key.c b/net/mac80211/debugfs_key.c
index 1521cab..ec40f27 100644
--- a/net/mac80211/debugfs_key.c
+++ b/net/mac80211/debugfs_key.c
@@ -299,11 +299,8 @@ void ieee80211_debugfs_key_update_default(struct ieee80211_sub_if_data *sdata)
return;

lockdep_assert_held(&sdata->local->key_mtx);
-
- if (sdata->debugfs.default_unicast_key) {
- debugfs_remove(sdata->debugfs.default_unicast_key);
- sdata->debugfs.default_unicast_key = NULL;
- }
+ debugfs_remove(sdata->debugfs.default_unicast_key);
+ sdata->debugfs.default_unicast_key = NULL;

if (sdata->default_unicast_key) {
key = key_mtx_dereference(sdata->local,
@@ -314,10 +311,8 @@ void ieee80211_debugfs_key_update_default(struct ieee80211_sub_if_data *sdata)
sdata->vif.debugfs_dir, buf);
}

- if (sdata->debugfs.default_multicast_key) {
- debugfs_remove(sdata->debugfs.default_multicast_key);
- sdata->debugfs.default_multicast_key = NULL;
- }
+ debugfs_remove(sdata->debugfs.default_multicast_key);
+ sdata->debugfs.default_multicast_key = NULL;

if (sdata->default_multicast_key) {
key = key_mtx_dereference(sdata->local,

[Johannes Berg]

Post by Fabian Frederick
WARNING: debugfs_remove(NULL) is safe this check is probably not required

I'll apply this; however, I think that checkpatch is a just tool, and
the commit message should reflect why you're changing the code.
Presumably you're not doing it to make the tool happy, but to address an
issue that the tool pointed out, so I think in most cases the commit
message should state the former, not the latter.

Note that in this particular case the NULL check check could be there to
avoid a memory write (which can be significant depending on the context)
so blindly doing what the tool suggested wouldn't always be a good idea.

johannes

[Fabian Frederick]

Post by Johannes Berg

=C2=A0 =C2=A0 =C2=A0WARNING: debugfs_remove(NULL) is safe this check=

is probably not

Post by Johannes Berg

required

I'll apply this; however, I think that checkpatch is a just tool, and
the commit message should reflect why you're changing the code.
Presumably you're not doing it to make the tool happy, but to address=

an

Post by Johannes Berg
issue that the tool pointed out, so I think in most cases the commit
message should state the former, not the latter.
Note that in this particular case the NULL check check could be there=

to

Post by Johannes Berg
avoid a memory write (which can be significant depending on the conte=

xt)

Post by Johannes Berg
so blindly doing what the tool suggested wouldn't always be a good id=

ea.

Post by Johannes Berg
johannes

Thanks Johannes,

Maybe you can replace commit message with:
"
This patch removes NULL check before debugfs_remove.
That function already does that check and is
only called during key management so we can add some memory writes.
"

I can also resubmit patch if necessary.

Regards,
=46abian

[Johannes Berg]

Post by Fabian Frederick
I can also resubmit patch if necessary.

No worries, I've already applied the patch (with a modified commit
message).

johannes