lib: fix coverity for unused variable
Checks
Commit Message
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(-)
Comments
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];
>
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];
> >
>
>
>
>
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.
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.
@@ -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];