From mboxrd@z Thu Jan 1 00:00:00 1970 From: Akhil Goyal Subject: Re: [PATCH 2/2] examples/ipsec-secgw: fix portmask option parsing Date: Thu, 21 Jun 2018 19:18:45 +0530 Message-ID: <36d2a552-e043-7451-ed66-70d13f0f6fe6@nxp.com> References: <1528208163-31560-1-git-send-email-konstantin.ananyev@intel.com> <1528208163-31560-2-git-send-email-konstantin.ananyev@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: radu.nicolau@intel.com To: Konstantin Ananyev , dev@dpdk.org Return-path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00081.outbound.protection.outlook.com [40.107.0.81]) by dpdk.org (Postfix) with ESMTP id 66C551B91E for ; Thu, 21 Jun 2018 15:49:05 +0200 (CEST) In-Reply-To: <1528208163-31560-2-git-send-email-konstantin.ananyev@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, On 6/5/2018 7:46 PM, Konstantin Ananyev wrote: > 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. This would also allow cryptodev_mask == 0 to work well which should not be the case. > > Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application") > > Signed-off-by: Konstantin Ananyev > --- > 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; > 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); I think there is no need for v, enabled_cryptodev_mask can be used instead. > if (ret == -1) { enabled_cryptodev_mask should not be 0 and should be checked here. -Akhil > - 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);