diff mbox

[dpdk-dev] app/testpmd: fix runtime dead code

Message ID 20170929125656.33164-1-michalx.k.jastrzebski@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Michal Jastrzebski Sept. 29, 2017, 12:56 p.m. UTC 
  From: Tomasz Kulasek <tomaszx.kulasek@intel.com>

Coverity reports DEADCODE, as assumes that RTE_LIBRTE_I40E_PMD
is defined and function compiles entirely. 
The fix is about to place printf function into #else branch
of conditional compilation, since it is known at compile
time if i40e is supported. No need to check it at runtime. 

Coverity issue: 158646
Fixes: 37a56cce7b8e ("app/testpmd: enable DDP get info feature")
Cc: andrey.chilikin@intel.com
Cc: stable@dpdk.org

Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
 app/test-pmd/cmdline.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Jingjing Wu Sept. 29, 2017, 2:25 p.m. UTC | #1 
> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Friday, September 29, 2017 8:57 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.jain@intel.com>; Kulasek, TomaszX
> <tomaszx.kulasek@intel.com>; Chilikin, Andrey <andrey.chilikin@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] app/testpmd: fix runtime dead code
> 
> From: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> 
> Coverity reports DEADCODE, as assumes that RTE_LIBRTE_I40E_PMD
> is defined and function compiles entirely.
> The fix is about to place printf function into #else branch
> of conditional compilation, since it is known at compile
> time if i40e is supported. No need to check it at runtime.
> 
> Coverity issue: 158646
> Fixes: 37a56cce7b8e ("app/testpmd: enable DDP get info feature")
> Cc: andrey.chilikin@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Ferruh Yigit Oct. 12, 2017, 7:28 p.m. UTC | #2 
On 9/29/2017 1:56 PM, Michal Jastrzebski wrote:
> From: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> 
> Coverity reports DEADCODE, as assumes that RTE_LIBRTE_I40E_PMD
> is defined and function compiles entirely. 
> The fix is about to place printf function into #else branch
> of conditional compilation, since it is known at compile
> time if i40e is supported. No need to check it at runtime. 

This is to highlight this is not supported other than defined PMD, and
used like this in similar functions, I am for keeping this and marking
coverity item as false positive.

Hi Jingjing,

What do you think, if you have strong opinion to get this, I will.

Thanks,
ferruh

> 
> Coverity issue: 158646
> Fixes: 37a56cce7b8e ("app/testpmd: enable DDP get info feature")
> Cc: andrey.chilikin@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
>  app/test-pmd/cmdline.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index ccdf239..c604dac 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -13414,8 +13414,8 @@ struct cmd_ddp_info_result {
>  	struct cmd_ddp_info_result *res = parsed_result;
>  	uint8_t *pkg;
>  	uint32_t pkg_size;
> -	int ret = -ENOTSUP;
>  #ifdef RTE_LIBRTE_I40E_PMD
> +	int ret;
>  	uint32_t i;
>  	uint8_t *buff;
>  	uint32_t buff_size;
> @@ -13495,10 +13495,9 @@ struct cmd_ddp_info_result {
>  			free(devs);
>  		}
>  	}
> -	ret = 0;
> +#else
> +	printf("Function not supported in PMD driver\n");
>  #endif
> -	if (ret == -ENOTSUP)
> -		printf("Function not supported in PMD driver\n");
>  	close_ddp_package_file(pkg);
>  }
>  
>
Jingjing Wu Oct. 13, 2017, 1:36 a.m. UTC | #3 
> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Friday, October 13, 2017 3:28 AM

> To: Jastrzebski, MichalX K <michalx.k.jastrzebski@intel.com>; Wu, Jingjing

> <jingjing.wu@intel.com>

> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.jain@intel.com>; Kulasek,

> TomaszX <tomaszx.kulasek@intel.com>; Chilikin, Andrey

> <andrey.chilikin@intel.com>; stable@dpdk.org

> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix runtime dead code

> 

> On 9/29/2017 1:56 PM, Michal Jastrzebski wrote:

> > From: Tomasz Kulasek <tomaszx.kulasek@intel.com>

> >

> > Coverity reports DEADCODE, as assumes that RTE_LIBRTE_I40E_PMD is

> > defined and function compiles entirely.

> > The fix is about to place printf function into #else branch of

> > conditional compilation, since it is known at compile time if i40e is

> > supported. No need to check it at runtime.

> 

> This is to highlight this is not supported other than defined PMD, and used like

> this in similar functions, I am for keeping this and marking coverity item as

> false positive.

> 

> Hi Jingjing,

> 

> What do you think, if you have strong opinion to get this, I will.

> 

> Thanks,

> ferruh

> 

> >


Hi, Ferruh

Checked other funcs called PMD specific API. And it seems all of them handle
That in the same way. So, to keep consistent, we may not need to apply this patch.

I agree with you to mark coverity item to no problem.

Thanks
Jingjing
diff mbox

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index ccdf239..c604dac 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -13414,8 +13414,8 @@  struct cmd_ddp_info_result {
 	struct cmd_ddp_info_result *res = parsed_result;
 	uint8_t *pkg;
 	uint32_t pkg_size;
-	int ret = -ENOTSUP;
 #ifdef RTE_LIBRTE_I40E_PMD
+	int ret;
 	uint32_t i;
 	uint8_t *buff;
 	uint32_t buff_size;
@@ -13495,10 +13495,9 @@  struct cmd_ddp_info_result {
 			free(devs);
 		}
 	}
-	ret = 0;
+#else
+	printf("Function not supported in PMD driver\n");
 #endif
-	if (ret == -ENOTSUP)
-		printf("Function not supported in PMD driver\n");
 	close_ddp_package_file(pkg);
 }