> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Tuesday 6 January 2026 18:27
> To: [email protected]
> Cc: Stephen Hemminger <[email protected]>
> 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;
> }
