Re: [PATCH v3 1/3] eal: add function rte_size_to_str

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Mar 12, 2025 at 12:28:32PM -0700, Andre Muezerie wrote:
> It's common to use %' in the printf format specifier to make large numbers
> more easily readable by having the thousands grouped. However, this
> grouping does not work on Windows. Therefore, a function is needed to make
> uint64_t numbers more easily readable. There are at least two tests that
> can benefit from this new function.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  lib/eal/common/eal_common_string_fns.c | 44 ++++++++++++++++++++++++++
>  lib/eal/include/rte_common.h           | 31 ++++++++++++++++++
>  lib/eal/version.map                    |  3 ++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/lib/eal/common/eal_common_string_fns.c b/lib/eal/common/eal_common_string_fns.c
> index 9ca2045b18..4cc7f35652 100644
> --- a/lib/eal/common/eal_common_string_fns.c
> +++ b/lib/eal/common/eal_common_string_fns.c
> @@ -4,6 +4,7 @@
>  
>  #include <ctype.h>
>  #include <errno.h>
> +#include <inttypes.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  
> @@ -87,6 +88,12 @@ rte_str_to_size(const char *str)
>  		endptr++; /* allow 1 space gap */
>  
>  	switch (*endptr) {
> +	case 'E': case 'e':
> +		size *= 1024; /* fall-through */
> +	case 'P': case 'p':
> +		size *= 1024; /* fall-through */
> +	case 'T': case 't':
> +		size *= 1024; /* fall-through */
>  	case 'G': case 'g':
>  		size *= 1024; /* fall-through */
>  	case 'M': case 'm':
> @@ -98,3 +105,40 @@ rte_str_to_size(const char *str)
>  	}
>  	return size;
>  }
> +
> +int
> +rte_size_to_str(char *buf, int buf_size,
> +		uint64_t count, bool use_iec)
> +{
> +	const char *prefix = "kMGTPE";
> +	const unsigned int base = use_iec ? 1024 : 1000;
> +	uint64_t powi = 1;
> +	uint16_t powj = 1;
> +	uint8_t precision = 2;
> +
> +	if (count < base)
> +		return snprintf(buf, buf_size, "%"PRIu64" ", count);
> +
> +	/* increase value by a factor of 1000/1024 and store
> +	 * if result is something a human can read
> +	 */
> +	for (;;) {
> +		powi *= base;
> +		if (count / base < powi)
> +			break;
> +
> +		if (!prefix[1])
> +			break;
> +		++prefix;
> +	}
> +
> +	/* try to guess a good number of digits for precision */
> +	for (; precision > 0; precision--) {
> +		powj *= 10;
> +		if (count / powi < powj)
> +			break;
> +	}
> +
> +	return snprintf(buf, buf_size, "%.*f %c%s", precision,
> +			(double)count / powi, *prefix, use_iec ? "i" : "");
> +}
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 386f11ae40..781c56adcd 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -14,9 +14,11 @@
>  
>  #include <assert.h>
>  #include <limits.h>
> +#include <stdbool.h>
>  #include <stdint.h>
>  #include <stdalign.h>
>  
> +#include <rte_compat.h>
>  #include <rte_config.h>
>  
>  /* OS specific include */
> @@ -919,6 +921,35 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
>  uint64_t
>  rte_str_to_size(const char *str);
>  
> +/**
> + * Converts the uint64_t value provided to a human-readable string.
> + * It null-terminates the string, truncating the data if needed.
> + *
> + * Sample outputs with "use_iec" disabled and enabled:
> + * 0 : "0 ", "0 "
> + * 700 : "700 ", "700 "
> + * 1000 : "1.00 k", "1000 "
> + * 1024 : "1.02 k", "1.00 ki"
> + * 21474836480 : "21.5 G", "20.0 Gi"
> + * 109951162777600 : "110 T", "100 Ti"
> + *
> + * @param buf
> + *     Buffer to write the string to.
> + * @param buf_size
> + *     Size of the buffer.
> + * @param count
> + *     Number to convert.
> + * @param use_iec
> + *     If true, use IEC units (1024-based), otherwise use SI units (1000-based).
> + * @return
> + *     Number of characters written (not including the null-terminator),
> + *     or that would have been required when the buffer is too small.

Just one further minor query on the API return value.

While this is a good return value for handling error cases, for
convenience I wonder if it might be better to have the function return
"buf" or NULL on error. For basic usage where the buffer is known to be
sufficiently big, that would allow use inline in printfs/logs, rather than
requiring a separate call e.g.:

printf("Value is %sB/s\n", rte_size_to_str(buf, RTE_DIM(buf), value, true));

WDYT?
/Bruce