On 08/11/2019 14:45, Xu, Rosen wrote: > Hi Kevin, > > Too many things in these days, sorry for late reply. >
Hi Rosen, no problem, thanks for the Ack. Kevin. >> -----Original Message----- >> From: Kevin Traynor [mailto:ktray...@redhat.com] >> Sent: Tuesday, November 05, 2019 23:42 >> To: David Marchand <david.march...@redhat.com>; Xu, Rosen >> <rosen...@intel.com> >> Cc: dev <dev@dpdk.org>; dpdk stable <sta...@dpdk.org>; Ye, Xiaolong >> <xiaolong...@intel.com> >> Subject: Re: [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check >> logic >> >> On 30/10/2019 07:59, David Marchand wrote: >>> Hello Rosen, >>> >>> Review please. >>> >> >> Ping Rosen. >> >>> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktray...@redhat.com> >> wrote: >>>> >>>> Coverity is complaining about identical code regardless of which >>>> branch of the if else is taken. Functionally it means an error will >>>> always be returned if this if else is hit. Remove the else branch. >>>> >>>> CID 337928 (#1 of 1): Identical code for different branches >>>> (IDENTICAL_BRANCHES)identical_branches: The same code is executed >>>> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS || >>>> n->n_children != 0U is true, because the 'then' and 'else' branches >>>> are identical. Should one of the branches be modified, or the entire >>>> 'if' statement replaced? > > Yes, you are right. > >>>> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS || >>>> 1507 n->n_children != 0) { >>>> 1508 return -rte_tm_error_set(error, >>>> 1509 EINVAL, >>>> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED, >>>> 1511 NULL, >>>> 1512 rte_strerror(EINVAL)); >>>> else_branch: The else branch, identical to the then branch. >>>> 1513 } else { >>>> 1514 return -rte_tm_error_set(error, >>>> 1515 EINVAL, >>>> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED, >>>> 1517 NULL, >>>> 1518 rte_strerror(EINVAL)); >>>> 1519 } >>>> >>>> Coverity issue: 337928 >>>> Fixes: c820468ac99c ("net/ipn3ke: support TM") >>>> Cc: rosen...@intel.com >>>> Cc: sta...@dpdk.org >>>> >>>> Signed-off-by: Kevin Traynor <ktray...@redhat.com> >>>> --- >>>> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------ >>>> 1 file changed, 6 deletions(-) >>>> >>>> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c >>>> b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644 >>>> --- a/drivers/net/ipn3ke/ipn3ke_tm.c >>>> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c >>>> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct >> rte_eth_dev *dev, >>>> NULL, >>>> rte_strerror(EINVAL)); >>>> - } else { >>>> - return -rte_tm_error_set(error, >>>> - EINVAL, >>>> - >>>> RTE_TM_ERROR_TYPE_UNSPECIFIED, >>>> - NULL, >>>> - rte_strerror(EINVAL)); >>>> } >>>> } >>>> -- >>>> 2.21.0 >>>> >>> > > Reviewed-by: Rosen Xu <rosen...@intel.com> >
