Hi Konstantin,
> -----Original Message-----
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Konstantin Ananyev
> Sent: Tuesday, June 5, 2018 3:16 PM
> To: dev@dpdk.org; dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; Nicolau, Radu
> <radu.nico...@intel.com>
> Subject: [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: fix portmask option
> parsing
>
> parse_portmask() returns both portmask value and possible error code as 32-bit
> integer. That causes some confusion for callers.
> Split error code and portmask value into two distinct variables.
> Also allows to run the app with unprotected_port_mask == 0.
>
> Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
>
> Signed-off-by: Konstantin Ananyev <konstantin.anan...@intel.com>
> ---
> examples/ipsec-secgw/ipsec-secgw.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> secgw/ipsec-secgw.c
> index fafb41161..5d7071657 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -972,20 +972,19 @@ print_usage(const char *prgname) }
>
> static int32_t
> -parse_portmask(const char *portmask)
> +parse_portmask(const char *portmask, uint32_t *pmv)
> {
> - char *end = NULL;
> + char *end;
> unsigned long pm;
>
> /* parse hexadecimal string */
> + errno = 0;
> pm = strtoul(portmask, &end, 16);
> - if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
> + if (errno != 0 || *end != '\0' || pm > UINT32_MAX)
> return -1;
>
> - if ((pm == 0) && errno)
> - return -1;
> -
> - return pm;
> + *pmv = pm;
> + return 0;
> }
>
> static int32_t
> @@ -1063,6 +1062,7 @@ parse_args(int32_t argc, char **argv)
> int32_t opt, ret;
> char **argvopt;
> int32_t option_index;
> + uint32_t v;
The variable "v" seems a bit cryptic to me, how about "pmv" or "mask_val" or
"value"?
> char *prgname = argv[0];
> int32_t f_present = 0;
>
> @@ -1073,8 +1073,8 @@ parse_args(int32_t argc, char **argv)
>
> switch (opt) {
> case 'p':
> - enabled_port_mask = parse_portmask(optarg);
> - if (enabled_port_mask == 0) {
> + ret = parse_portmask(optarg, &enabled_port_mask);
> + if (ret < 0 || enabled_port_mask == 0) {
> printf("invalid portmask\n");
> print_usage(prgname);
> return -1;
> @@ -1085,8 +1085,8 @@ parse_args(int32_t argc, char **argv)
> promiscuous_on = 1;
> break;
> case 'u':
> - unprotected_port_mask = parse_portmask(optarg);
> - if (unprotected_port_mask == 0) {
> + ret = parse_portmask(optarg,
> &unprotected_port_mask);
> + if (ret < 0) {
> printf("invalid unprotected portmask\n");
> print_usage(prgname);
> return -1;
> @@ -1147,15 +1147,16 @@ parse_args(int32_t argc, char **argv)
> single_sa_idx);
> break;
> case CMD_LINE_OPT_CRYPTODEV_MASK_NUM:
> - ret = parse_portmask(optarg);
> + ret = parse_portmask(optarg, &v);
> if (ret == -1) {
> - printf("Invalid argument[portmask]\n");
> + printf("Invalid argument[%s]\n",
> + CMD_LINE_OPT_CRYPTODEV_MASK);
> print_usage(prgname);
> return -1;
> }
>
> /* else */
> - enabled_cryptodev_mask = ret;
> + enabled_cryptodev_mask = v;
> break;
> default:
> print_usage(prgname);
> --
> 2.13.6
Regards,
Bernard.
