diff mbox series

app: fix redundant comparison

Message ID 1618835391-54882-1-git-send-email-humin29@huawei.com (mailing list archive)
State New
Delegated to: David Marchand
Headers show
Series app: fix redundant comparison | expand

Checks

Context Check Description
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/github-robot success github build: passed
ci/travis-robot success travis build: passed
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Min Hu (Connor) April 19, 2021, 12:29 p.m. UTC
The return value of 'rte_eal_cleanup' is always zero, so comparison
with zero is redundant.

This patch fixed it by deleting the redundant comparison.

Fixes: 67684d1e87b6 ("app/procinfo: call EAL cleanup before exit")
Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
Fixes: 5e516c89830a ("app/testpmd: call cleanup on exit")
Fixes: 613ce6691c0d ("examples/l3fwd-power: implement proper shutdown")
Fixes: 48be180de631 ("eal: move OS common debug functions to single file")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/pdump/main.c                         | 4 +---
 app/proc-info/main.c                     | 4 +---
 app/test-compress-perf/main.c            | 7 +------
 app/test-pmd/testpmd.c                   | 5 +----
 examples/l3fwd-power/main.c              | 3 +--
 lib/librte_eal/common/eal_common_debug.c | 4 +---
 6 files changed, 6 insertions(+), 21 deletions(-)

Comments

David Hunt April 19, 2021, 12:44 p.m. UTC | #1
Hi Connor,

On 19/4/2021 1:29 PM, Min Hu (Connor) wrote:
> The return value of 'rte_eal_cleanup' is always zero, so comparison
> with zero is redundant.
>
> This patch fixed it by deleting the redundant comparison.
>
> Fixes: 67684d1e87b6 ("app/procinfo: call EAL cleanup before exit")
> Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
> Fixes: 5e516c89830a ("app/testpmd: call cleanup on exit")
> Fixes: 613ce6691c0d ("examples/l3fwd-power: implement proper shutdown")
> Fixes: 48be180de631 ("eal: move OS common debug functions to single file")
> Cc: stable@dpdk.org
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>   app/pdump/main.c                         | 4 +---
>   app/proc-info/main.c                     | 4 +---
>   app/test-compress-perf/main.c            | 7 +------
>   app/test-pmd/testpmd.c                   | 5 +----
>   examples/l3fwd-power/main.c              | 3 +--
>   lib/librte_eal/common/eal_common_debug.c | 4 +---
>   6 files changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index 63bbe65..33523c5 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -1014,9 +1014,7 @@ main(int argc, char **argv)
>   	/* dump debug stats */
>   	print_pdump_stats();
>   
> -	ret = rte_eal_cleanup();
> -	if (ret)
> -		printf("Error from rte_eal_cleanup(), %d\n", ret);
> +	rte_eal_cleanup();
>   
>   	return 0;
>   }
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index b9587f7..9498490 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -1458,9 +1458,7 @@ main(int argc, char **argv)
>   	RTE_ETH_FOREACH_DEV(i)
>   		rte_eth_dev_close(i);
>   
> -	ret = rte_eal_cleanup();
> -	if (ret)
> -		printf("Error from rte_eal_cleanup(), %d\n", ret);
> +	rte_eal_cleanup();
>   
>   	return 0;
>   }
> diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
> index cc9951a..03ee385 100644
> --- a/app/test-compress-perf/main.c
> +++ b/app/test-compress-perf/main.c
> @@ -474,12 +474,7 @@ main(int argc, char **argv)
>   		/* fallthrough */
>   	case ST_CLEAR:
>   	default:
> -		i = rte_eal_cleanup();
> -		if (i) {
> -			RTE_LOG(ERR, USER1,
> -				"Error from rte_eal_cleanup(), %d\n", i);
> -			ret = i;
> -		}
> +		rte_eal_cleanup();
>   		break;
>   	}
>   	return ret;
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index d4be23f..1f8a7b8 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3972,10 +3972,7 @@ main(int argc, char** argv)
>   			return 1;
>   	}
>   
> -	ret = rte_eal_cleanup();
> -	if (ret != 0)
> -		rte_exit(EXIT_FAILURE,
> -			 "EAL cleanup failed: %s\n", strerror(-ret));
> +	rte_eal_cleanup();
>   
>   	return EXIT_SUCCESS;
>   }
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index 0af8810..84464d5 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -2914,8 +2914,7 @@ main(int argc, char **argv)
>   			deinit_power_library())
>   		rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
>   
> -	if (rte_eal_cleanup() < 0)
> -		RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n");
> +	rte_eal_cleanup();
>   
>   	return 0;
>   }
> diff --git a/lib/librte_eal/common/eal_common_debug.c b/lib/librte_eal/common/eal_common_debug.c
> index 15418e9..04b1fed 100644
> --- a/lib/librte_eal/common/eal_common_debug.c
> +++ b/lib/librte_eal/common/eal_common_debug.c
> @@ -37,8 +37,6 @@ rte_exit(int exit_code, const char *format, ...)
>   	rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
>   	va_end(ap);
>   
> -	if (rte_eal_cleanup() != 0)
> -		RTE_LOG(CRIT, EAL,
> -			"EAL could not release all resources\n");
> +	rte_eal_cleanup();
>   	exit(exit_code);
>   }


Since the function is defined as "int rte_eal_cleanup(void)" and the 
documentation says:

  *  - 0 Successfully released all internal EAL resources.
  *  - -EFAULT There was an error in releasing all resources.

We really should check for non zero values. Just because it's currently 
coded to only return zero may not hold in the future. The code may 
change in the future to return non-zero values.

Rgds,
Dave.
Stephen Hemminger April 19, 2021, 3:06 p.m. UTC | #2
On Mon, 19 Apr 2021 20:29:51 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> The return value of 'rte_eal_cleanup' is always zero, so comparison
> with zero is redundant.
> 
> This patch fixed it by deleting the redundant comparison.
> 
> Fixes: 67684d1e87b6 ("app/procinfo: call EAL cleanup before exit")
> Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
> Fixes: 5e516c89830a ("app/testpmd: call cleanup on exit")
> Fixes: 613ce6691c0d ("examples/l3fwd-power: implement proper shutdown")
> Fixes: 48be180de631 ("eal: move OS common debug functions to single file")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Please keep it in testpmd, which should be checking everything.
For the others, the message doesn't add any value to the user and can be removed.
diff mbox series

Patch

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 63bbe65..33523c5 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -1014,9 +1014,7 @@  main(int argc, char **argv)
 	/* dump debug stats */
 	print_pdump_stats();
 
-	ret = rte_eal_cleanup();
-	if (ret)
-		printf("Error from rte_eal_cleanup(), %d\n", ret);
+	rte_eal_cleanup();
 
 	return 0;
 }
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b9587f7..9498490 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1458,9 +1458,7 @@  main(int argc, char **argv)
 	RTE_ETH_FOREACH_DEV(i)
 		rte_eth_dev_close(i);
 
-	ret = rte_eal_cleanup();
-	if (ret)
-		printf("Error from rte_eal_cleanup(), %d\n", ret);
+	rte_eal_cleanup();
 
 	return 0;
 }
diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
index cc9951a..03ee385 100644
--- a/app/test-compress-perf/main.c
+++ b/app/test-compress-perf/main.c
@@ -474,12 +474,7 @@  main(int argc, char **argv)
 		/* fallthrough */
 	case ST_CLEAR:
 	default:
-		i = rte_eal_cleanup();
-		if (i) {
-			RTE_LOG(ERR, USER1,
-				"Error from rte_eal_cleanup(), %d\n", i);
-			ret = i;
-		}
+		rte_eal_cleanup();
 		break;
 	}
 	return ret;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d4be23f..1f8a7b8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3972,10 +3972,7 @@  main(int argc, char** argv)
 			return 1;
 	}
 
-	ret = rte_eal_cleanup();
-	if (ret != 0)
-		rte_exit(EXIT_FAILURE,
-			 "EAL cleanup failed: %s\n", strerror(-ret));
+	rte_eal_cleanup();
 
 	return EXIT_SUCCESS;
 }
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 0af8810..84464d5 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2914,8 +2914,7 @@  main(int argc, char **argv)
 			deinit_power_library())
 		rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
 
-	if (rte_eal_cleanup() < 0)
-		RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n");
+	rte_eal_cleanup();
 
 	return 0;
 }
diff --git a/lib/librte_eal/common/eal_common_debug.c b/lib/librte_eal/common/eal_common_debug.c
index 15418e9..04b1fed 100644
--- a/lib/librte_eal/common/eal_common_debug.c
+++ b/lib/librte_eal/common/eal_common_debug.c
@@ -37,8 +37,6 @@  rte_exit(int exit_code, const char *format, ...)
 	rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
 	va_end(ap);
 
-	if (rte_eal_cleanup() != 0)
-		RTE_LOG(CRIT, EAL,
-			"EAL could not release all resources\n");
+	rte_eal_cleanup();
 	exit(exit_code);
 }