* [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask
@ 2020-06-11 12:36 Sarosh Arif
2020-07-11 10:00 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2020-07-21 16:07 ` [dpdk-dev] " Bruce Richardson
0 siblings, 2 replies; 4+ messages in thread
From: Sarosh Arif @ 2020-06-11 12:36 UTC (permalink / raw)
To: dev, drc, maxime.coquelin, zhihong.wang, xiaolong.ye,
bruce.richardson, konstantin.ananyev, david.hunt, jerinj, skori,
john.mcnamara, kirill.rybalchenko
Cc: stable, 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.
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(-)
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;
}
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] examples: fix return value of function that parses portmask
2020-06-11 12:36 [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask Sarosh Arif
@ 2020-07-11 10:00 ` Thomas Monjalon
2020-07-21 16:07 ` [dpdk-dev] " Bruce Richardson
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2020-07-11 10:00 UTC (permalink / raw)
To: Sarosh Arif
Cc: dev, drc, maxime.coquelin, zhihong.wang, xiaolong.ye,
bruce.richardson, konstantin.ananyev, david.hunt, jerinj, skori,
john.mcnamara, kirill.rybalchenko, stable, david.marchand
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask
2020-06-11 12:36 [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask Sarosh Arif
2020-07-11 10:00 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
@ 2020-07-21 16:07 ` Bruce Richardson
2020-07-30 21:08 ` Thomas Monjalon
1 sibling, 1 reply; 4+ messages in thread
From: Bruce Richardson @ 2020-07-21 16:07 UTC (permalink / raw)
To: Sarosh Arif
Cc: dev, drc, maxime.coquelin, zhihong.wang, xiaolong.ye,
konstantin.ananyev, david.hunt, jerinj, skori, john.mcnamara,
kirill.rybalchenko, stable
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask
2020-07-21 16:07 ` [dpdk-dev] " Bruce Richardson
@ 2020-07-30 21:08 ` Thomas Monjalon
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2020-07-30 21:08 UTC (permalink / raw)
To: Sarosh Arif
Cc: dev, drc, maxime.coquelin, zhihong.wang, xiaolong.ye,
konstantin.ananyev, david.hunt, jerinj, skori, john.mcnamara,
kirill.rybalchenko, stable, Bruce Richardson
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-30 21:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 12:36 [dpdk-dev] [PATCH] examples: fix return value of function that parses portmask Sarosh Arif
2020-07-11 10:00 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2020-07-21 16:07 ` [dpdk-dev] " Bruce Richardson
2020-07-30 21:08 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).