Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Pablo Neira Ayuso]

On Sun, Apr 09, 2017 at 09:12:18AM +0530, Arushi Singhal wrote:
> On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso 
> wrote:
> 
> > On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
> > > On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
> > >
> > > >Replace explicit NULL comparison with ! operator to simplify code.
> > >
> > > I still wouldn't do this, for the same reason as before. Comparing to
> > > NULL explicitly more or less gave an extra guarantee that the other
> > > operand was also a pointer.
> >
> > Arushi, where does it say in the coding style that this is prefered?
> 
> This is reported by checkpatch.pl script.

I don't find it in the coding style. I think this is what it stands as
preference in this case IMO. Otherwise, it would be good to get the
kernel coding style document in sync with it, including the reason why
this way to express thing is cleaner. We have to justify the changes.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Liping Zhang]

2017-04-09 16:26 GMT+08:00 Jan Engelhardt :
>
> On Sunday 2017-04-09 05:42, Arushi Singhal wrote:
>>On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso  wrote:
>>  On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
>>  > On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
>>  >
>>  > >Replace explicit NULL comparison with ! operator to simplify code.
>>  >
>>  > I still wouldn't do this, for the same reason as before. Comparing to
>>  > NULL explicitly more or less gave an extra guarantee that the other
>>  > operand was also a pointer.
>>
>>  Arushi, where does it say in the coding style that this is prefered?
>>
>>This is reported by checkpatch.pl script.
>
> checkpatch has been controversial at times, like when people took the 80
> character limit way too literally. Changing pointer comparisons looks like
> another thing that is better left ignored.

Yes, I agree too. Converting the "if (p != NULL)" to "if (p)" like this seems
unnecessary.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Jan Engelhardt]

On Sunday 2017-04-09 05:42, Arushi Singhal wrote:
>On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso  wrote:
>  On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
>  > On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
>  >
>  > >Replace explicit NULL comparison with ! operator to simplify code.
>  >
>  > I still wouldn't do this, for the same reason as before. Comparing to
>  > NULL explicitly more or less gave an extra guarantee that the other
>  > operand was also a pointer.
>
>  Arushi, where does it say in the coding style that this is prefered? 
>
>This is reported by checkpatch.pl script.

checkpatch has been controversial at times, like when people took the 80
character limit way too literally. Changing pointer comparisons looks like
another thing that is better left ignored.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Pablo Neira Ayuso]

On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
> On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
> 
> >Replace explicit NULL comparison with ! operator to simplify code.
> 
> I still wouldn't do this, for the same reason as before. Comparing to 
> NULL explicitly more or less gave an extra guarantee that the other 
> operand was also a pointer.

Arushi, where does it say in the coding style that this is prefered?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

[Jan Engelhardt]

On Saturday 2017-04-08 19:21, Arushi Singhal wrote:

>Replace explicit NULL comparison with ! operator to simplify code.

I still wouldn't do this, for the same reason as before. Comparing to 
NULL explicitly more or less gave an extra guarantee that the other 
operand was also a pointer.

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: netfilter: Replace explicit NULL comparisons

[Arushi Singhal]

Replace explicit NULL comparison with ! operator to simplify code.

Signed-off-by: Arushi Singhal 
---
 net/netfilter/nf_conntrack_broadcast.c |  2 +-
 net/netfilter/nf_conntrack_core.c  |  2 +-
 net/netfilter/nf_conntrack_ecache.c|  4 +--
 net/netfilter/nf_conntrack_helper.c|  4 +--
 net/netfilter/nf_conntrack_proto.c |  4 +--
 net/netfilter/nf_log.c |  2 +-
 net/netfilter/nf_nat_redirect.c|  2 +-
 net/netfilter/nf_tables_api.c  | 62 +-
 net/netfilter/nfnetlink_log.c  |  6 ++--
 net/netfilter/nfnetlink_queue.c|  8 ++---
 net/netfilter/nft_compat.c |  4 +--
 net/netfilter/nft_ct.c | 10 +++---
 net/netfilter/nft_dynset.c | 14 
 net/netfilter/nft_log.c| 14 
 net/netfilter/nft_lookup.c |  2 +-
 net/netfilter/nft_payload.c|  4 +--
 net/netfilter/nft_set_hash.c   |  4 +--
 net/netfilter/x_tables.c   |  8 ++---
 net/netfilter/xt_TCPMSS.c  |  4 +--
 net/netfilter/xt_addrtype.c|  2 +-
 net/netfilter/xt_connlimit.c   |  2 +-
 net/netfilter/xt_conntrack.c   |  2 +-
 net/netfilter/xt_hashlimit.c   |  4 +--
 net/netfilter/xt_recent.c  |  6 ++--
 24 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/net/netfilter/nf_conntrack_broadcast.c 
b/net/netfilter/nf_conntrack_broadcast.c
index 4e99cca61612..a016d47e5a80 100644
--- a/net/netfilter/nf_conntrack_broadcast.c
+++ b/net/netfilter/nf_conntrack_broadcast.c
@@ -42,7 +42,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
 
rcu_read_lock();
in_dev = __in_dev_get_rcu(rt->dst.dev);
-   if (in_dev != NULL) {
+   if (in_dev) {
for_primary_ifa(in_dev) {
if (ifa->ifa_broadcast == iph->daddr) {
mask = ifa->ifa_mask;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index ffb78e5f7b70..282d7ec1acba 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1345,7 +1345,7 @@ nf_conntrack_in(struct net *net, u_int8_t pf, unsigned 
int hooknum,
/* It may be an special packet, error, unclean...
 * inverse of the return code tells to the netfilter
 * core what to do with the packet. */
-   if (l4proto->error != NULL) {
+   if (l4proto->error) {
ret = l4proto->error(net, tmpl, skb, dataoff, pf, hooknum);
if (ret <= 0) {
NF_CT_STAT_INC_ATOMIC(net, error);
diff --git a/net/netfilter/nf_conntrack_ecache.c 
b/net/netfilter/nf_conntrack_ecache.c
index da9df2d56e66..11184cae5329 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -266,7 +266,7 @@ int nf_conntrack_register_notifier(struct net *net,
mutex_lock(_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
   
lockdep_is_held(_ct_ecache_mutex));
-   if (notify != NULL) {
+   if (notify) {
ret = -EBUSY;
goto out_unlock;
}
@@ -302,7 +302,7 @@ int nf_ct_expect_register_notifier(struct net *net,
mutex_lock(_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
   
lockdep_is_held(_ct_ecache_mutex));
-   if (notify != NULL) {
+   if (notify) {
ret = -EBUSY;
goto out_unlock;
}
diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 6dc44d9b4190..fda6348a88e5 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -224,9 +224,9 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct 
nf_conn *tmpl,
if (test_bit(IPS_HELPER_BIT, >status))
return 0;
 
-   if (tmpl != NULL) {
+   if (tmpl) {
help = nfct_help(tmpl);
-   if (help != NULL) {
+   if (help) {
helper = help->helper;
set_bit(IPS_HELPER_BIT, >status);
}
diff --git a/net/netfilter/nf_conntrack_proto.c 
b/net/netfilter/nf_conntrack_proto.c
index 2d6ee1803415..cb1e1593fc82 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -307,7 +307,7 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
int err = 0;
 
 #ifdef CONFIG_SYSCTL
-   if (pn->ctl_table != NULL) {
+   if (pn->ctl_table) {
err = nf_ct_register_sysctl(net,
>ctl_table_header,
"net/netfilter",
@@ -329,7 +329,7 @@ void nf_ct_l4proto_unregister_sysctl(struct net *net,