Re: [PATCH][V2] i40evf: remove redundant array comparisons to 0 checks

[Andy Shevchenko]

On Mon, Feb 19, 2018 at 12:23 PM, Colin King  wrote:
> From: Colin Ian King 
>
> The checks to see if key->dst.s6_addr and key->src.s6_addr are null
> pointers are redundant because these are constant size arrays and
> so the checks always return true.  Fix this by removing the redundant
> checks.   Also replace filter->f with vf, allowing wide lines to be
> condensed and to rejoin some split wide lines.
>
> Detected by CoverityScan, CID#1465279 ("Array compared to 0")
>
> Signed-off-by: Colin Ian King 
> ---

Changelog?


> u16 addr_type = 0;
> u16 n_proto = 0;
> int i = 0;
> +   struct virtchnl_filter *vf = &filter->f;
>

Perhaps perserve reversed xmas tree order?


> -   filter->f.flow_type = VIRTCHNL_TCP_V6_FLOW;
> +   vf->flow_type = VIRTCHNL_TCP_V6_FLOW;

No need to touch.

> }
>
> if (key->ip_proto != IPPROTO_TCP) {
> @@ -2585,9 +2586,8 @@ static int i40evf_parse_cls_flower(struct 
> i40evf_adapter *adapter,
> is_multicast_ether_addr(key->dst)) {
> /* set the mask if a valid dst_mac address */
> for (i = 0; i < ETH_ALEN; i++)
> -   filter->f.mask.tcp_spec.dst_mac[i] |=
> -   0xff;
> -   
> ether_addr_copy(filter->f.data.tcp_spec.dst_mac,
> +   vf->mask.tcp_spec.dst_mac[i] |= 0xff;
> +   ether_addr_copy(vf->data.tcp_spec.dst_mac,
> key->dst);

Instead, keep two pointers:
 ... *pointer_mask = &vf->mask.tcp_spec;
 ... *pointer_data = &vf->data.tcp_spec;


-- 
With Best Regards,
Andy Shevchenko

[PATCH][V2] i40evf: remove redundant array comparisons to 0 checks

[Colin King]

From: Colin Ian King 

The checks to see if key->dst.s6_addr and key->src.s6_addr are null
pointers are redundant because these are constant size arrays and
so the checks always return true.  Fix this by removing the redundant
checks.   Also replace filter->f with vf, allowing wide lines to be
condensed and to rejoin some split wide lines.

Detected by CoverityScan, CID#1465279 ("Array compared to 0")

Signed-off-by: Colin Ian King 
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 61 ++---
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c 
b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 4955ce3ab6a2..a13df27089c3 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2493,6 +2493,7 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter 
*adapter,
u16 addr_type = 0;
u16 n_proto = 0;
int i = 0;
+   struct virtchnl_filter *vf = &filter->f;
 
if (f->dissector->used_keys &
~(BIT(FLOW_DISSECTOR_KEY_CONTROL) |
@@ -2540,7 +2541,7 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter 
*adapter,
return -EINVAL;
if (n_proto == ETH_P_IPV6) {
/* specify flow type as TCP IPv6 */
-   filter->f.flow_type = VIRTCHNL_TCP_V6_FLOW;
+   vf->flow_type = VIRTCHNL_TCP_V6_FLOW;
}
 
if (key->ip_proto != IPPROTO_TCP) {
@@ -2585,9 +2586,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter 
*adapter,
is_multicast_ether_addr(key->dst)) {
/* set the mask if a valid dst_mac address */
for (i = 0; i < ETH_ALEN; i++)
-   filter->f.mask.tcp_spec.dst_mac[i] |=
-   0xff;
-   ether_addr_copy(filter->f.data.tcp_spec.dst_mac,
+   vf->mask.tcp_spec.dst_mac[i] |= 0xff;
+   ether_addr_copy(vf->data.tcp_spec.dst_mac,
key->dst);
}
 
@@ -2596,9 +2596,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter 
*adapter,
is_multicast_ether_addr(key->src)) {
/* set the mask if a valid dst_mac address */
for (i = 0; i < ETH_ALEN; i++)
-   filter->f.mask.tcp_spec.src_mac[i] |=
-   0xff;
-   ether_addr_copy(filter->f.data.tcp_spec.src_mac,
+   vf->mask.tcp_spec.src_mac[i] |= 0xff;
+   ether_addr_copy(vf->data.tcp_spec.src_mac,
key->src);
}
}
@@ -2622,8 +2621,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter 
*adapter,
return I40E_ERR_CONFIG;
}
}
-   filter->f.mask.tcp_spec.vlan_id |= cpu_to_be16(0x);
-   filter->f.data.tcp_spec.vlan_id = cpu_to_be16(key->vlan_id);
+   vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0x);
+   vf->data.tcp_spec.vlan_id = cpu_to_be16(key->vlan_id);
}
 
if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_CONTROL)) {
@@ -2670,14 +2669,12 @@ static int i40evf_parse_cls_flower(struct 
i40evf_adapter *adapter,
return I40E_ERR_CONFIG;
}
if (key->dst) {
-   filter->f.mask.tcp_spec.dst_ip[0] |=
-   cpu_to_be32(0x);
-   filter->f.data.tcp_spec.dst_ip[0] = key->dst;
+   vf->mask.tcp_spec.dst_ip[0] |= cpu_to_be32(0x);
+   vf->data.tcp_spec.dst_ip[0] = key->dst;
}
if (key->src) {
-   filter->f.mask.tcp_spec.src_ip[0] |=
-   cpu_to_be32(0x);
-   filter->f.data.tcp_spec.src_ip[0] = key->src;
+   vf->mask.tcp_spec.src_ip[0] |= cpu_to_be32(0x);
+   vf->data.tcp_spec.src_ip[0] = key->src;
}
}
 
@@ -2710,22 +2707,14 @@ static int i40evf_parse_cls_flower(struct 
i40evf_adapter *adapter,
if (!ipv6_addr_any(&mask->dst) || !ipv6_addr_any(&mask->src))
field_flags |= I40EVF_CLOUD_FIELD_IIP;
 
-   if (key->dst.s6_addr) {
-   for (i = 0; i < 4; i++)
-