diff mbox series

examples: fix return value of function that parses portmask

Message ID 20200611123624.25319-1-sarosh.arif@emumba.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series examples: fix return value of function that parses portmask |

Checks

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

Commit Message

Sarosh Arif June 11, 2020, 12:36 p.m. UTC 
Giving invalid or zero portmask as command line option to 
these applications will have an unexpected response.
The reason behind this is that the return value of function
that parses portmask is stored in a variable whose datatype is 
unsigned int, hence returning -1 in case of zero or
invalid portmask causes an unexpected behaviour. 
If we return 0 instead of -1 this issue can be resolved.
The program already contains the functionality to print
"invalid portmask" and program usage if zero is returned.

Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
 examples/distributor/main.c                     | 5 +----
 examples/ioat/ioatfwd.c                         | 2 +-
 examples/ip_reassembly/main.c                   | 5 +----
 examples/l2fwd-event/main.c                     | 5 +----
 examples/l2fwd-jobstats/main.c                  | 5 +----
 examples/l2fwd-keepalive/main.c                 | 5 +----
 examples/l2fwd/main.c                           | 5 +----
 examples/l3fwd-acl/main.c                       | 5 +----
 examples/l3fwd-graph/main.c                     | 5 +----
 examples/l3fwd-power/main.c                     | 5 +----
 examples/l3fwd/main.c                           | 5 +----
 examples/link_status_interrupt/main.c           | 5 +----
 examples/performance-thread/l3fwd-thread/main.c | 5 +----
 examples/ptpclient/ptpclient.c                  | 5 +----
 examples/qos_meter/main.c                       | 5 +----
 examples/tep_termination/main.c                 | 5 +----
 examples/vhost/main.c                           | 5 +----
 examples/vm_power_manager/main.c                | 5 +----
 examples/vmdq/main.c                            | 5 +----
 examples/vmdq_dcb/main.c                        | 5 +----
 20 files changed, 20 insertions(+), 77 deletions(-)

Comments

Thomas Monjalon July 11, 2020, 10 a.m. UTC | #1 
Why nobody reviewed? Isn't there some maintainers of example apps?

11/06/2020 14:36, Sarosh Arif:
> Giving invalid or zero portmask as command line option to 
> these applications will have an unexpected response.
> The reason behind this is that the return value of function
> that parses portmask is stored in a variable whose datatype is 
> unsigned int, hence returning -1 in case of zero or
> invalid portmask causes an unexpected behaviour. 

After looking at few examples, the function returns a signed int.

> If we return 0 instead of -1 this issue can be resolved.

Yes, the caller of the function seems to expect 0 as error value.

> The program already contains the functionality to print
> "invalid portmask" and program usage if zero is returned.
> 
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
Bruce Richardson July 21, 2020, 4:07 p.m. UTC | #2 
On Thu, Jun 11, 2020 at 05:36:24PM +0500, Sarosh Arif wrote:
> Giving invalid or zero portmask as command line option to 
> these applications will have an unexpected response.
> The reason behind this is that the return value of function
> that parses portmask is stored in a variable whose datatype is 
> unsigned int, hence returning -1 in case of zero or
> invalid portmask causes an unexpected behaviour. 
> If we return 0 instead of -1 this issue can be resolved.
> The program already contains the functionality to print
> "invalid portmask" and program usage if zero is returned.
> 
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---

Checked a number of the examples and all seem to behave similarly to
described. This looks a good fix.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Thomas Monjalon July 30, 2020, 9:08 p.m. UTC | #3 
21/07/2020 18:07, Bruce Richardson:
> On Thu, Jun 11, 2020 at 05:36:24PM +0500, Sarosh Arif wrote:
> > Giving invalid or zero portmask as command line option to 
> > these applications will have an unexpected response.
> > The reason behind this is that the return value of function
> > that parses portmask is stored in a variable whose datatype is 
> > unsigned int, hence returning -1 in case of zero or
> > invalid portmask causes an unexpected behaviour. 
> > If we return 0 instead of -1 this issue can be resolved.
> > The program already contains the functionality to print
> > "invalid portmask" and program usage if zero is returned.
> > 
> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > ---
> 
> Checked a number of the examples and all seem to behave similarly to
> described. This looks a good fix.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks
diff mbox series

Patch

diff --git a/examples/distributor/main.c b/examples/distributor/main.c
index 567c5e989..dca48c2ab 100644
--- a/examples/distributor/main.c
+++ b/examples/distributor/main.c
@@ -647,10 +647,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c
index 53de23179..863dc0454 100644
--- a/examples/ioat/ioatfwd.c
+++ b/examples/ioat/ioatfwd.c
@@ -582,7 +582,7 @@  ioat_parse_portmask(const char *portmask)
 	/* Parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index 494d7ee77..550fb53be 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -580,10 +580,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l2fwd-event/main.c b/examples/l2fwd-event/main.c
index 9593ef11e..951a02106 100644
--- a/examples/l2fwd-event/main.c
+++ b/examples/l2fwd-event/main.c
@@ -39,10 +39,7 @@  l2fwd_event_parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 396fd89db..275407419 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -562,10 +562,7 @@  l2fwd_parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
index b7585d55e..8d4a65990 100644
--- a/examples/l2fwd-keepalive/main.c
+++ b/examples/l2fwd-keepalive/main.c
@@ -305,10 +305,7 @@  l2fwd_parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
index f8d14b843..a99b7558a 100644
--- a/examples/l2fwd/main.c
+++ b/examples/l2fwd/main.c
@@ -311,10 +311,7 @@  l2fwd_parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index f22fca732..112ec3d2c 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -1567,10 +1567,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l3fwd-graph/main.c b/examples/l3fwd-graph/main.c
index c70270c4d..d3fcf411c 100644
--- a/examples/l3fwd-graph/main.c
+++ b/examples/l3fwd-graph/main.c
@@ -302,10 +302,7 @@  parse_portmask(const char *portmask)
 	/* Parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 9db94ce04..9dde35c7d 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1470,10 +1470,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 24ede4290..de6c62293 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -340,10 +340,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/link_status_interrupt/main.c b/examples/link_status_interrupt/main.c
index 25efe2b09..a6b4aa1dc 100644
--- a/examples/link_status_interrupt/main.c
+++ b/examples/link_status_interrupt/main.c
@@ -312,10 +312,7 @@  lsi_parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/performance-thread/l3fwd-thread/main.c b/examples/performance-thread/l3fwd-thread/main.c
index 84c1d7b3a..e32802aa9 100644
--- a/examples/performance-thread/l3fwd-thread/main.c
+++ b/examples/performance-thread/l3fwd-thread/main.c
@@ -2681,10 +2681,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index bfa86eec5..20da32517 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -650,10 +650,7 @@  ptp_parse_portmask(const char *portmask)
 	pm = strtoul(portmask, &end, 16);
 
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
index 6d057abfe..ce87b2eca 100644
--- a/examples/qos_meter/main.c
+++ b/examples/qos_meter/main.c
@@ -220,10 +220,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index ab956ad7c..4cb102119 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -203,10 +203,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index ab649bf14..690b50f85 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -407,10 +407,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0') || (errno != 0))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 
diff --git a/examples/vm_power_manager/main.c b/examples/vm_power_manager/main.c
index 273bfec29..ef9c0684a 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -144,10 +144,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/vmdq/main.c b/examples/vmdq/main.c
index d08826c86..660be4011 100644
--- a/examples/vmdq/main.c
+++ b/examples/vmdq/main.c
@@ -370,10 +370,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }
diff --git a/examples/vmdq_dcb/main.c b/examples/vmdq_dcb/main.c
index f417b2fd9..83a6843ee 100644
--- a/examples/vmdq_dcb/main.c
+++ b/examples/vmdq_dcb/main.c
@@ -424,10 +424,7 @@  parse_portmask(const char *portmask)
 	/* parse hexadecimal string */
 	pm = strtoul(portmask, &end, 16);
 	if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
-		return -1;
-
-	if (pm == 0)
-		return -1;
+		return 0;
 
 	return pm;
 }