Re: [PATCH] net: netfilters: Remove extra parenthesis

[Pablo Neira Ayuso]

Hi Arushi,

On Sat, Mar 25, 2017 at 07:23:13PM +0530, Arushi Singhal wrote:
> diff --git a/net/netfilter/nf_conntrack_netlink.c 
> b/net/netfilter/nf_conntrack_netlink.c
> index 6806b5e73567..aa344c5868c5 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -467,7 +467,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32 
> seq, u32 type,
>   struct nlattr *nest_parms;
>   unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>  
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW;

Could you send us a unfront patch to add something like:

static inline u16 nfnl_msg_type(u8 subsys, u8 msg_type)
{
return subsys << 8 | msg_type;
}

I would suggest you place this in include/linux/netfilter/nfnetlink.h

Then, use it here.

>   nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
>   if (nlh == NULL)
>   goto nlmsg_failure;
> @@ -1983,7 +1983,7 @@ ctnetlink_ct_stat_cpu_fill_info(struct sk_buff *skb, 
> u32 portid, u32 seq,
>   struct nfgenmsg *nfmsg;
>   unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>  
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU;

... And here too.

>   nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
>   if (nlh == NULL)
>   goto nlmsg_failure;
> @@ -2066,7 +2066,7 @@ ctnetlink_stat_ct_fill_info(struct sk_buff *skb, u32 
> portid, u32 seq, u32 type,
>   unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>   unsigned int nr_conntracks = atomic_read(>ct.count);
>  
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS;

And so on. Look for more spots where we can replace this opencoded
thing.

I guess there are more spots in all of the net/netfilter/ netlink
subsystems.

Once that patch gets in, you can follow up with this parens cleanup.

Thanks!

P.S: Cc'ing netfilter-de...@vger.kernel.org (and
lvs-de...@vger.kernel.org if you touch 'ipvs' bits) should be fine. No
need to Cc all those many lists, better to narrow down you target.

[PATCH] net: netfilters: Remove extra parenthesis

[Arushi Singhal]

Rmoved parentheses on the right hand side of assignment, as they are
not required. The following coccinelle script was used to fix this
issue:

@@
local idexpression id;
expression e;
@@

id =
-(
e
-)

Signed-off-by: Arushi Singhal 
---
 net/netfilter/ipvs/ip_vs_proto_tcp.c   | 2 +-
 net/netfilter/nf_conntrack_netlink.c   | 8 
 net/netfilter/nf_conntrack_proto_tcp.c | 2 +-
 net/netfilter/nft_fib.c| 2 +-
 net/netfilter/nft_set_bitmap.c | 2 +-
 net/netfilter/x_tables.c   | 2 +-
 net/netfilter/xt_socket.c  | 8 
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c 
b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index 5117bcb7d2f0..9f2abda017c4 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -487,7 +487,7 @@ static struct tcp_states_t tcp_states_dos [] = {
 
 static void tcp_timeout_change(struct ip_vs_proto_data *pd, int flags)
 {
-   int on = (flags & 1);   /* secure_tcp */
+   int on = flags & 1; /* secure_tcp */
 
/*
** FIXME: change secure_tcp to independent sysctl var
diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index 6806b5e73567..aa344c5868c5 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -467,7 +467,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32 
seq, u32 type,
struct nlattr *nest_parms;
unsigned int flags = portid ? NLM_F_MULTI : 0, event;
 
-   event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW);
+   event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
if (nlh == NULL)
goto nlmsg_failure;
@@ -1983,7 +1983,7 @@ ctnetlink_ct_stat_cpu_fill_info(struct sk_buff *skb, u32 
portid, u32 seq,
struct nfgenmsg *nfmsg;
unsigned int flags = portid ? NLM_F_MULTI : 0, event;
 
-   event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU);
+   event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
if (nlh == NULL)
goto nlmsg_failure;
@@ -2066,7 +2066,7 @@ ctnetlink_stat_ct_fill_info(struct sk_buff *skb, u32 
portid, u32 seq, u32 type,
unsigned int flags = portid ? NLM_F_MULTI : 0, event;
unsigned int nr_conntracks = atomic_read(>ct.count);
 
-   event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS);
+   event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
if (nlh == NULL)
goto nlmsg_failure;
@@ -3223,7 +3223,7 @@ ctnetlink_exp_stat_fill_info(struct sk_buff *skb, u32 
portid, u32 seq, int cpu,
struct nfgenmsg *nfmsg;
unsigned int flags = portid ? NLM_F_MULTI : 0, event;
 
-   event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_EXP_GET_STATS_CPU);
+   event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_EXP_GET_STATS_CPU;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
if (nlh == NULL)
goto nlmsg_failure;
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c 
b/net/netfilter/nf_conntrack_proto_tcp.c
index b122e9dacfed..2e5e4a4d4778 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -789,7 +789,7 @@ static int tcp_error(struct net *net, struct nf_conn *tmpl,
}
 
/* Check TCP flags. */
-   tcpflags = (tcp_flag_byte(th) & ~(TCPHDR_ECE|TCPHDR_CWR|TCPHDR_PSH));
+   tcpflags = tcp_flag_byte(th) & ~(TCPHDR_ECE|TCPHDR_CWR|TCPHDR_PSH);
if (!tcp_valid_flags[tcpflags]) {
if (LOG_INVALID(net, IPPROTO_TCP))
nf_log_packet(net, pf, 0, skb, NULL, NULL, NULL,
diff --git a/net/netfilter/nft_fib.c b/net/netfilter/nft_fib.c
index 29a4906adc27..c907c27b0374 100644
--- a/net/netfilter/nft_fib.c
+++ b/net/netfilter/nft_fib.c
@@ -35,7 +35,7 @@ int nft_fib_validate(const struct nft_ctx *ctx, const struct 
nft_expr *expr,
switch (priv->result) {
case NFT_FIB_RESULT_OIF: /* fallthrough */
case NFT_FIB_RESULT_OIFNAME:
-   hooks = (1 << NF_INET_PRE_ROUTING);
+   hooks = 1 << NF_INET_PRE_ROUTING;
break;
case NFT_FIB_RESULT_ADDRTYPE:
if (priv->flags & NFTA_FIB_F_IIF)
diff --git a/net/netfilter/nft_set_bitmap.c b/net/netfilter/nft_set_bitmap.c
index 152d226552c1..09867146539e 100644
--- a/net/netfilter/nft_set_bitmap.c
+++ b/net/netfilter/nft_set_bitmap.c
@@ -47,7 +47,7 @@ struct nft_bitmap {
 
 static inline void nft_bitmap_location(u32 key, u32 *idx, u32 *off)
 {
-   u32 k = (key << 1);
+   u32 k = key << 1;
 
*idx = k /