[24.03,2/4] arg_parser: add new coremask parsing API

  Add new coremask parsing API. This API behaves similarly to the corelist parsing
API, parsing the coremask string, filling its values into the cores array.

The API also returns a 'count' which corresponds to the total number of cores
in the coremask string.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 lib/arg_parser/arg_parser.c     | 58 +++++++++++++++++++++++++++++++++
 lib/arg_parser/rte_arg_parser.h | 33 +++++++++++++++++++
 lib/arg_parser/version.map      |  1 +
 3 files changed, 92 insertions(+)
  

On Wed, Nov 22, 2023 at 04:45:48PM +0000, Euan Bourke wrote:
> Add new coremask parsing API. This API behaves similarly to the corelist parsing
> API, parsing the coremask string, filling its values into the cores array.
> 

General tip - commit log messages are generally wrapped at 72 characters.

> The API also returns a 'count' which corresponds to the total number of cores
> in the coremask string.
> 
> Signed-off-by: Euan Bourke <euan.bourke@intel.com>

Again, some review comments inline below.

/Bruce

> ---
>  lib/arg_parser/arg_parser.c     | 58 +++++++++++++++++++++++++++++++++
>  lib/arg_parser/rte_arg_parser.h | 33 +++++++++++++++++++
>  lib/arg_parser/version.map      |  1 +
>  3 files changed, 92 insertions(+)
> 
> diff --git a/lib/arg_parser/arg_parser.c b/lib/arg_parser/arg_parser.c
> index 45acaf5631..58be94b67d 100644
> --- a/lib/arg_parser/arg_parser.c
> +++ b/lib/arg_parser/arg_parser.c
> @@ -11,6 +11,9 @@
>  #include <rte_arg_parser.h>
>  #include <rte_common.h>
>  
> +#define BITS_PER_HEX 4
> +#define MAX_COREMASK_SIZE ((UINT16_MAX+1)/BITS_PER_HEX)
> +

Whitespace around "/" operator here, and below in bits definition (which I
missed on review of first patch).

>  
>  struct core_bits {
>  	uint8_t bits[(UINT16_MAX + 1)/CHAR_BIT];
> @@ -57,6 +60,15 @@ corebits_to_array(struct core_bits *mask, uint16_t *cores, size_t max_cores)
>  	}
>  }
>  
> +static int xdigit2val(unsigned char c)
> +{
> +	if (isdigit(c))
> +		return c - '0';
> +	else if (isupper(c))
> +		return c - 'A' + 10;
> +	else
> +		return c - 'a' + 10;
> +}
>  
>  int
>  rte_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len)
> @@ -111,3 +123,49 @@ rte_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len)
>  
>  	return total_count;
>  }
> +
> +int
> +rte_parse_coremask(const char *coremask, uint16_t *cores, uint32_t cores_len)
> +{
> +	struct core_bits *mask = malloc(sizeof(struct core_bits));

Check return value from malloc. Need to do so in patch 1 also.

> +	memset(mask, 0, sizeof(struct core_bits));
> +
> +	/* Remove all blank characters ahead and after .
> +	 * Remove 0x/0X if exists.
> +	 */
> +	while (isblank(*coremask))
> +		coremask++;
> +	if (coremask[0] == '0' && ((coremask[1] == 'x')
> +			|| (coremask[1] == 'X')))

Nit: this can all fit on one line, as it's <100 chars long.

> +		coremask += 2;
> +
> +	int32_t i = strlen(coremask);
> +	while ((i > 0) && isblank(coremask[i - 1]))
> +		i--;
> +	if (i == 0 || i > MAX_COREMASK_SIZE)
> +		return -1;
> +
> +	uint32_t idx = 0;
> +	uint8_t j;
> +	int val;
> +

You can define "val" inside the for loop as it's not needed outside it.
Since we use C11 standard, you can also avoid declaring j here too, and
just do "for (uint8_t j = 0; ....)".

> +	for (i = i - 1; i >= 0; i--) {
> +		char c = coremask[i];
> +
> +		if (isxdigit(c) == 0)
> +			return -1;
> +
> +		val = xdigit2val(c);
> +
> +		for (j = 0; j < BITS_PER_HEX; j++, idx++) {
> +			if ((1 << j) & val)
> +				set_core_bit(mask, idx);
> +		}
> +	}
> +
> +	corebits_to_array(mask, cores, cores_len);
> +	uint32_t total_count = mask->total_bits_set;
> +	free(mask);
> +
> +	return total_count;
> +}
> diff --git a/lib/arg_parser/rte_arg_parser.h b/lib/arg_parser/rte_arg_parser.h
> index 1b12bf451f..b149b37755 100644
> --- a/lib/arg_parser/rte_arg_parser.h
> +++ b/lib/arg_parser/rte_arg_parser.h
> @@ -58,6 +58,39 @@ __rte_experimental
>  int
>  rte_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len);
>  
> +/**
> + * Convert a string describing a bitmask of core ids into an array of core ids.
> + *
> + * On success, the passed array is filled with the core ids present in the
> + * bitmask up to the "cores_len", and the number of elements added into the array is returned.
> + * For example, passing a 0xA "coremask" results in an array of [1, 3]
> + * and would return 2.
> + * 
> + * Like the snprintf function for strings, if the length of the input array is
> + * insufficient to hold the number of cores in the "coresmask", the input array is
> + * filled to capacity and the return value is the number of elements which would
> + * be returned if the array had been big enough.
> + * Function can also be called with a NULL array and 0 "cores_len" to find out
> + * the "cores_len" required.
> + *
> + * @param coremask
> + *   A string containing a bitmask of core ids.
> + * @param cores
> + *   An array where to store the core ids.
> + *   Array can be NULL if "cores_len" is 0.
> + * @param cores_len
> + *   The length of the "cores" array.
> + *   If the size is smaller than that needed to hold all cores from "coremask",
> + *   only "cores_len" elements will be written to the array.
> + * @return
> + *   n: the number of unique cores present in "coremask".
> + *   -1 if the string was invalid.
> + *   NOTE: if n > "cores_len", then only "cores_len" elements in the "cores" array are valid.
> +*/
> +__rte_experimental
> +int
> +rte_parse_coremask(const char* coremask, uint16_t *cores, uint32_t cores_len);
> +
>  
>  #ifdef __cplusplus
>  }
> diff --git a/lib/arg_parser/version.map b/lib/arg_parser/version.map
> index f11699a306..f334f1aaed 100644
> --- a/lib/arg_parser/version.map
> +++ b/lib/arg_parser/version.map
> @@ -7,4 +7,5 @@ EXPERIMENTAL {
>  
>      # added in 24.03
>      rte_parse_corelist;
> +    rte_parse_coremask;
>  };
> -- 
> 2.34.1
>