RE: [PATCH 03/12] net/pcap: use bool for flags

[Marat Khalili <marat.khalili@huawei.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday 6 January 2026 18:27
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [PATCH 03/12] net/pcap: use bool for flags
> 
> Save some space by using bool for flag values.
> Use common code to parse value of a boolean flag.

Note that there might technically be some performance cost of this. Said that, 
since original code did not leave any comments why ints were used, let's think 
it was not on purpose, while bools are cleaner and less error-prone.

I however think that proper bool conversion should use true or false on 
assignment/initialization, not 1 or 0.

// snip

> @@ -1180,31 +1181,29 @@ open_tx_iface(const char *key, const char *value, void *extra_args)
>  }
> 
>  static int
> -select_phy_mac(const char *key __rte_unused, const char *value,
> -		void *extra_args)
> +process_bool_flag(const char *key, const char *value, void *extra_args)

This function probably belongs in some common library. There is a similar code 
in lib/argparse/rte_argparse.c and lib/cmdline/cmdline_parse_bool.c that might 
benefit. Having some tests could also be useful.

I would also think of naming more, word "process" does not really convey much 
meaning. How about "parse_bool"?

>  {
> -	if (extra_args) {
> -		const int phy_mac = atoi(value);
> -		int *enable_phy_mac = extra_args;
> -
> -		if (phy_mac)
> -			*enable_phy_mac = 1;
> -	}
> -	return 0;
> -}
> +	bool *flag = extra_args;
> +	/* table of possible representation of boolean */
> +	static const char * const values[] = {
> +		"false", "true",
> +		"0", "1",
> +		"disable", "enable",
> +		"on", "off",

Last pair is swapped.

> +	};
> 
> -static int
> -get_infinite_rx_arg(const char *key __rte_unused,
> -		const char *value, void *extra_args)
> -{
> -	if (extra_args) {
> -		const int infinite_rx = atoi(value);
> -		int *enable_infinite_rx = extra_args;
> +	for (unsigned int i = 0; i < RTE_DIM(values); i++) {
> +		if (strcmp(value, values[i]) == 0) {

Maybe strcasecmp (just suggesting)?

> +			*flag = !!(i & 1);
> 
> -		if (infinite_rx > 0)
> -			*enable_infinite_rx = 1;
> +			PMD_LOG(INFO, "%s set to %s",
> +				key, *flag ? "true" : "false");

Should we have a common use {"false", "true"} inline function somewhere? 
Logging of infinite_rx in the same file could use it, as well as a few other 
places.

> +			return 0;
> +		}
>  	}
> -	return 0;
> +
> +	PMD_LOG(ERR, "Invalid '%s' value '%s'", key, value);
> +	return -1;
>  }