[PATCH] app/pdump: close program if --pdump argument is missing

[PATCH] app/pdump: close program if --pdump argument is missing

[usman.tanveer]

--pdump is a mandatory argument in pdump application.
It should print usage and exit if --pdump argument
is missing. The application is not closing and getting
stuck. Made the change to print usage and exit when
this argument is missing.

Signed-off-by: usman.tanveer <usman.tanveer@emumba.com>
---
 app/pdump/main.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 04a38e8911..59a6846c65 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -383,14 +383,17 @@ launch_args_parse(int argc, char **argv, char *prgname)
 {
 	int opt, ret;
 	int option_index;
+	bool pdump_flag = false;
 	static struct option long_option[] = {
 		{CMD_LINE_OPT_PDUMP, 1, 0, CMD_LINE_OPT_PDUMP_NUM},
 		{CMD_LINE_OPT_MULTI, 0, 0, CMD_LINE_OPT_MULTI_NUM},
 		{NULL, 0, 0, 0}
 	};
 
-	if (argc == 1)
+	if (argc == 1) {
 		pdump_usage(prgname);
+		return -1;
+	}
 
 	/* Parse command line */
 	while ((opt = getopt_long(argc, argv, " ",
@@ -402,6 +405,7 @@ launch_args_parse(int argc, char **argv, char *prgname)
 				pdump_usage(prgname);
 				return -1;
 			}
+			pdump_flag = true;
 			break;
 		case CMD_LINE_OPT_MULTI_NUM:
 			multiple_core_capture = 1;
@@ -412,6 +416,11 @@ launch_args_parse(int argc, char **argv, char *prgname)
 		}
 	}
 
+	if (pdump_flag == false) {
+		pdump_usage(prgname);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -991,11 +1000,9 @@ main(int argc, char **argv)
 	argv += (diag - 2);
 
 	/* parse app arguments */
-	if (argc > 1) {
-		ret = launch_args_parse(argc, argv, argp[0]);
-		if (ret < 0)
-			rte_exit(EXIT_FAILURE, "Invalid argument\n");
-	}
+	ret = launch_args_parse(argc, argv, argp[0]);
+	if (ret < 0)
+		rte_exit(EXIT_FAILURE, "Invalid argument\n");
 
 	/* create mempool, ring and vdevs info */
 	create_mp_ring_vdev();
-- 
2.25.1

Re: [PATCH] app/pdump: close program if --pdump argument is missing

[Stephen Hemminger]

On Wed,  2 Mar 2022 14:26:14 +0500
"usman.tanveer" <usman.tanveer@emumba.com> wrote:

> --pdump is a mandatory argument in pdump application.
> It should print usage and exit if --pdump argument
> is missing. The application is not closing and getting
> stuck. Made the change to print usage and exit when
> this argument is missing.
> 
> Signed-off-by: usman.tanveer <usman.tanveer@emumba.com>

Maybe pdump could just do something sane with no arguments.
That is one of the issues I tried to address by rewriting it.

Ideally, existing pdump can be deprecated and removed in 22.11?

Re: [PATCH] app/pdump: close program if --pdump argument is missing

[Usman Tanveer]

The 20.11 release is not coming out till August, in the meantime, this
patch can be applied to print help if mandatory argument is not
passed.


On Wed, Mar 2, 2022 at 9:52 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed,  2 Mar 2022 14:26:14 +0500
> "usman.tanveer" <usman.tanveer@emumba.com> wrote:
>
> > --pdump is a mandatory argument in pdump application.
> > It should print usage and exit if --pdump argument
> > is missing. The application is not closing and getting
> > stuck. Made the change to print usage and exit when
> > this argument is missing.
> >
> > Signed-off-by: usman.tanveer <usman.tanveer@emumba.com>
>
> Maybe pdump could just do something sane with no arguments.
> That is one of the issues I tried to address by rewriting it.
>
> Ideally, existing pdump can be deprecated and removed in 22.11?

[PATCH v2] app/pdump: exit if no device specified

[Stephen Hemminger]

Simpler version of earlier patch which had a good idea, was just
implemented with more code than necessary.
If no device is specified don't start the capture loop.

Reported-by: usman.tanveer <usman.tanveer@emumba.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/pdump/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index c94606275b28..7a1c7bdf6011 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -915,6 +915,9 @@ dump_packets(void)
 	int i;
 	unsigned int lcore_id = 0;
 
+	if (num_tuples == 0)
+		rte_exit(EXIT_FAILURE, "No device specified for capture\n");
+
 	if (!multiple_core_capture) {
 		printf(" core (%u), capture for (%d) tuples\n",
 				rte_lcore_id(), num_tuples);
-- 
2.39.2

Re: [PATCH v2] app/pdump: exit if no device specified

[fengchengwen]

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/7/1 10:16, Stephen Hemminger wrote:
> Simpler version of earlier patch which had a good idea, was just
> implemented with more code than necessary.
> If no device is specified don't start the capture loop.
> 
> Reported-by: usman.tanveer <usman.tanveer@emumba.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  app/pdump/main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index c94606275b28..7a1c7bdf6011 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -915,6 +915,9 @@ dump_packets(void)
>  	int i;
>  	unsigned int lcore_id = 0;
>  
> +	if (num_tuples == 0)
> +		rte_exit(EXIT_FAILURE, "No device specified for capture\n");
> +
>  	if (!multiple_core_capture) {
>  		printf(" core (%u), capture for (%d) tuples\n",
>  				rte_lcore_id(), num_tuples);
> 

Re: [PATCH v2] app/pdump: exit if no device specified

[Thomas Monjalon]

03/07/2023 08:29, fengchengwen:
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> 
> On 2023/7/1 10:16, Stephen Hemminger wrote:
> > Simpler version of earlier patch which had a good idea, was just
> > implemented with more code than necessary.
> > If no device is specified don't start the capture loop.
> > 
> > Reported-by: usman.tanveer <usman.tanveer@emumba.com>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks.

[PATCH] app/pdump: close program if --pdump argument is missing

[usman.tanveer]

--pdump is a mandatory argument in pdump application.
It should print usage and exit if --pdump argument
is missing. The application is not closing and geting
stuck. Made the change to print usage and exit when
this argument is missing.

Signed-off-by: usman.tanveer <usman.tanveer@emumba.com>
---
 app/pdump/main.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 04a38e8911..59a6846c65 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -383,14 +383,17 @@ launch_args_parse(int argc, char **argv, char *prgname)
 {
 	int opt, ret;
 	int option_index;
+	bool pdump_flag = false;
 	static struct option long_option[] = {
 		{CMD_LINE_OPT_PDUMP, 1, 0, CMD_LINE_OPT_PDUMP_NUM},
 		{CMD_LINE_OPT_MULTI, 0, 0, CMD_LINE_OPT_MULTI_NUM},
 		{NULL, 0, 0, 0}
 	};
 
-	if (argc == 1)
+	if (argc == 1) {
 		pdump_usage(prgname);
+		return -1;
+	}
 
 	/* Parse command line */
 	while ((opt = getopt_long(argc, argv, " ",
@@ -402,6 +405,7 @@ launch_args_parse(int argc, char **argv, char *prgname)
 				pdump_usage(prgname);
 				return -1;
 			}
+			pdump_flag = true;
 			break;
 		case CMD_LINE_OPT_MULTI_NUM:
 			multiple_core_capture = 1;
@@ -412,6 +416,11 @@ launch_args_parse(int argc, char **argv, char *prgname)
 		}
 	}
 
+	if (pdump_flag == false) {
+		pdump_usage(prgname);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -991,11 +1000,9 @@ main(int argc, char **argv)
 	argv += (diag - 2);
 
 	/* parse app arguments */
-	if (argc > 1) {
-		ret = launch_args_parse(argc, argv, argp[0]);
-		if (ret < 0)
-			rte_exit(EXIT_FAILURE, "Invalid argument\n");
-	}
+	ret = launch_args_parse(argc, argv, argp[0]);
+	if (ret < 0)
+		rte_exit(EXIT_FAILURE, "Invalid argument\n");
 
 	/* create mempool, ring and vdevs info */
 	create_mp_ring_vdev();
-- 
2.25.1