[PATCH] lib: fix coverity for unused variable

[PATCH] lib: fix coverity for unused variable

[Amit Prakash Shukla]

Removed variable "rc" that stored a return value from a function
rte_lpm6_lookup_bulk_func.

Coverity issue: 385595
Fixes: 20365d793e45 ("node: add IPv6 lookup node")

Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
 lib/node/ip6_lookup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/node/ip6_lookup.c b/lib/node/ip6_lookup.c
index e4bbc7ed67..2b20194295 100644
--- a/lib/node/ip6_lookup.c
+++ b/lib/node/ip6_lookup.c
@@ -135,7 +135,7 @@ ip6_lookup_node_process_scalar(struct rte_graph *graph, struct rte_node *node,
 		node_mbuf_priv1(mbuf3, dyn)->ttl = ipv6_hdr->hop_limits;
 		rte_memcpy(ip_batch[3], ipv6_hdr->dst_addr, 16);
 
-		rc = rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop, 4);
+		rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop, 4);
 
 		next_hop[0] = (next_hop[0] < 0) ? (int32_t)drop_nh : next_hop[0];
 		node_mbuf_priv1(mbuf0, dyn)->nh = (uint16_t)next_hop[0];
-- 
2.25.1

Re: [PATCH] lib: fix coverity for unused variable

[Thomas Monjalon]

16/06/2023 13:06, Amit Prakash Shukla:
> Removed variable "rc" that stored a return value from a function
> rte_lpm6_lookup_bulk_func.

You are not really removing the variable, right?
It looks you just stop storing the return of this function.
Whouldn't it better to check the return code?


> Coverity issue: 385595
> Fixes: 20365d793e45 ("node: add IPv6 lookup node")
> 
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
>  lib/node/ip6_lookup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/node/ip6_lookup.c b/lib/node/ip6_lookup.c
> index e4bbc7ed67..2b20194295 100644
> --- a/lib/node/ip6_lookup.c
> +++ b/lib/node/ip6_lookup.c
> @@ -135,7 +135,7 @@ ip6_lookup_node_process_scalar(struct rte_graph *graph, struct rte_node *node,
>  		node_mbuf_priv1(mbuf3, dyn)->ttl = ipv6_hdr->hop_limits;
>  		rte_memcpy(ip_batch[3], ipv6_hdr->dst_addr, 16);
>  
> -		rc = rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop, 4);
> +		rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop, 4);
>  
>  		next_hop[0] = (next_hop[0] < 0) ? (int32_t)drop_nh : next_hop[0];
>  		node_mbuf_priv1(mbuf0, dyn)->nh = (uint16_t)next_hop[0];
> 

RE: [EXT] Re: [PATCH] lib: fix coverity for unused variable

[Amit Prakash Shukla]

Hi Thomas,

Please find my reply inline.

Thanks,
Amit Shukla

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, June 27, 2023 7:15 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>
> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Pavan Nikhilesh
> Bhagavatula <pbhagavatula@marvell.com>; dev@dpdk.org; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] Re: [PATCH] lib: fix coverity for unused variable
> 
> External Email
> 
> ----------------------------------------------------------------------
> 16/06/2023 13:06, Amit Prakash Shukla:
> > Removed variable "rc" that stored a return value from a function
> > rte_lpm6_lookup_bulk_func.
> 
> You are not really removing the variable, right?
> It looks you just stop storing the return of this function.
> Whouldn't it better to check the return code?

Yes, I am just not storing the return value. The reason to not
store return code is to avoid return code check in datapath.
rte_lpm6_lookup_bulk_func will return error only if
 lpm6, ip_batch, next_hop are NULL. In function ip6_lookup_node_process_scalar
these variables will never be NULL.

> 
> 
> > Coverity issue: 385595
> > Fixes: 20365d793e45 ("node: add IPv6 lookup node")
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > ---
> >  lib/node/ip6_lookup.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/node/ip6_lookup.c b/lib/node/ip6_lookup.c index
> > e4bbc7ed67..2b20194295 100644
> > --- a/lib/node/ip6_lookup.c
> > +++ b/lib/node/ip6_lookup.c
> > @@ -135,7 +135,7 @@ ip6_lookup_node_process_scalar(struct rte_graph
> *graph, struct rte_node *node,
> >  		node_mbuf_priv1(mbuf3, dyn)->ttl = ipv6_hdr->hop_limits;
> >  		rte_memcpy(ip_batch[3], ipv6_hdr->dst_addr, 16);
> >
> > -		rc = rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop,
> 4);
> > +		rte_lpm6_lookup_bulk_func(lpm6, ip_batch, next_hop, 4);
> >
> >  		next_hop[0] = (next_hop[0] < 0) ? (int32_t)drop_nh :
> next_hop[0];
> >  		node_mbuf_priv1(mbuf0, dyn)->nh =
> (uint16_t)next_hop[0];
> >
> 
> 
> 
> 

Re: [EXT] Re: [PATCH] lib: fix coverity for unused variable

[Thomas Monjalon]

27/06/2023 18:05, Amit Prakash Shukla:
> Hi Thomas,
> 
> Please find my reply inline.
> 
> Thanks,
> Amit Shukla
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, June 27, 2023 7:15 PM
> > To: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Pavan Nikhilesh
> > Bhagavatula <pbhagavatula@marvell.com>; dev@dpdk.org; Jerin Jacob
> > Kollanukkaran <jerinj@marvell.com>
> > Subject: [EXT] Re: [PATCH] lib: fix coverity for unused variable
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > 16/06/2023 13:06, Amit Prakash Shukla:
> > > Removed variable "rc" that stored a return value from a function
> > > rte_lpm6_lookup_bulk_func.
> > 
> > You are not really removing the variable, right?
> > It looks you just stop storing the return of this function.
> > Whouldn't it better to check the return code?
> 
> Yes, I am just not storing the return value. The reason to not
> store return code is to avoid return code check in datapath.
> rte_lpm6_lookup_bulk_func will return error only if
>  lpm6, ip_batch, next_hop are NULL. In function ip6_lookup_node_process_scalar
> these variables will never be NULL.

I will update the commit message accordingly.

Re: [EXT] Re: [PATCH] lib: fix coverity for unused variable

[Thomas Monjalon]

27/06/2023 23:52, Thomas Monjalon:
> 27/06/2023 18:05, Amit Prakash Shukla:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 16/06/2023 13:06, Amit Prakash Shukla:
> > > > Removed variable "rc" that stored a return value from a function
> > > > rte_lpm6_lookup_bulk_func.
> > > 
> > > You are not really removing the variable, right?
> > > It looks you just stop storing the return of this function.
> > > Whouldn't it better to check the return code?
> > 
> > Yes, I am just not storing the return value. The reason to not
> > store return code is to avoid return code check in datapath.
> > rte_lpm6_lookup_bulk_func will return error only if
> >  lpm6, ip_batch, next_hop are NULL. In function ip6_lookup_node_process_scalar
> > these variables will never be NULL.
> 
> I will update the commit message accordingly.

Applied with updated explanation:

    node: remove useless return code assignment
    
    No need to store the return code of rte_lpm6_lookup_bulk_func()
    as it returns error only if parameters are NULL.
    In the function ip6_lookup_node_process_scalar(),
    these variables will never be NULL.
    
    Not checking the return code will avoid check in datapath.
    Storing of the return code is useless and removed.