| Message ID | 1413908412-5177-1-git-send-email-fabf@skynet.be |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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(-)