* [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove
@ 2014-10-21 16:20 Fabian Frederick
2014-10-21 19:06 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Fabian Frederick @ 2014-10-21 16:20 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Johannes Berg, John W. Linville,
David S. Miller, linux-wireless, netdev
Fix checkpatch warnings:
WARNING: debugfs_remove(NULL) is safe this check is probably not required
Signed-off-by: Fabian Frederick <fabf@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,
--
1.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove
2014-10-21 16:20 [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove Fabian Frederick
@ 2014-10-21 19:06 ` Johannes Berg
2014-10-21 20:05 ` Fabian Frederick
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2014-10-21 19:06 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-kernel, John W. Linville, David S. Miller, linux-wireless, netdev
On Tue, 2014-10-21 at 18:20 +0200, Fabian Frederick wrote:
> Fix checkpatch warnings:
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove
2014-10-21 19:06 ` Johannes Berg
@ 2014-10-21 20:05 ` Fabian Frederick
2014-10-22 6:36 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Fabian Frederick @ 2014-10-21 20:05 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless, netdev, linux-kernel, David S. Miller, John W. Linville
> On 21 October 2014 at 21:06 Johannes Berg <johannes@sipsolutions.net> wrote:
>
>
> On Tue, 2014-10-21 at 18:20 +0200, Fabian Frederick wrote:
> > Fix checkpatch warnings:
> >
> > 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
>
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,
Fabian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove
2014-10-21 20:05 ` Fabian Frederick
@ 2014-10-22 6:36 ` Johannes Berg
0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2014-10-22 6:36 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-wireless, netdev, linux-kernel, David S. Miller, John W. Linville
On Tue, 2014-10-21 at 22:05 +0200, Fabian Frederick wrote:
> I can also resubmit patch if necessary.
No worries, I've already applied the patch (with a modified commit
message).
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-10-22 6:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21 16:20 [PATCH 1/1 net-next] mac80211: remove unnecessary null test before debugfs_remove Fabian Frederick
2014-10-21 19:06 ` Johannes Berg
2014-10-21 20:05 ` Fabian Frederick
2014-10-22 6:36 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).