On 16-Nov-18 10:15 AM, Burakov, Anatoly wrote:
On 07-Nov-18 6:10 AM, Li Han wrote:
parse_portmask return type is int,but global variable
"enabled_port_mask" type is uint32_t.so in proc_info_parse_args
function,when parse_portmask return -1,"enabled_port_mask" will
get a huge value and "if (enabled_port_mask == 0)" will never happen.
Fixes: 22561383ea17 ("app: replace dump_cfg by proc_info")
Signed-off-by: Li Han <han....@zte.com.cn>
---
v3:
*fix commit meassges issue
v2:
*fix typecast issue
---
app/proc-info/main.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index c20effa..650d599 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -37,7 +37,7 @@
#define MAX_STRING_LEN 256
/**< mask of enabled ports */
-static uint32_t enabled_port_mask;
+static uint64_t enabled_port_mask;
/**< Enable stats. */
static uint32_t enable_stats;
/**< Enable xstats. */
@@ -90,7 +90,7 @@
/*
* Parse the portmask provided at run time.
*/
-static int
+static unsigned long
parse_portmask(const char *portmask)
{
char *end = NULL;
@@ -103,12 +103,9 @@
if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0') ||
(errno != 0)) {
printf("%s ERROR parsing the port mask\n", __func__);
- return -1;
+ return 0;
}
- if (pm == 0)
- return -1;
-
return pm;
}
Hi,
This fix appears wrong. If you're making the value uint64_t, you cannot
encode errors in the value. So, it's better to leave the return type as
int, return 0 or -1 on success/error, and store the parsed result in a
pointer passed to the function instead. Something like this:
static int
parse_portmask(const char *portmask, uint64_t *mask)
{
...
if (pm == 0)
return -1;
*mask = pm;
return 0;
}
Also, another thing to note. Unsigned long is 32-bit on 32-bit Linux, so
if you're going to have uint64_t data, you should correct the parsing to
use strtoull() instead.
--
Thanks,
Anatoly
