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 <andre...@linux.microsoft.com>
> ---
Thanks Andre, comments inline below.
/Bruce
> 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)
Minor nit, I don't think you should need to wrap this, we can have lines up
to 100 chars long.
> +{
> + const char *prefix = "kMGTPE";
Why is "k" in lower case compared to the others all in upper-case?
Also, these are suffixes not prefixes. :-)
> + 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)
This would seem more logical to me as "count / powi < base" since it would
match the initial check for "count < base" (which is essentially the same
check since powi == 1).
> + break;
> +
> + if (!prefix[1])
Since prefix is character string, the comparison should be against '\0',
according to DPDK coding style. The "!" should only be used on bool values.
So "if (prefix[1] == '\0')" or "if (*(prefix + 1) == '\0')".
> + 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"
> + *
I would omit the space before the suffixes in the output. As well as
looking better to me, it also solves the issue of the non-suffixed numbers
having a trailing space.
> + * @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.
> + */
> +__rte_experimental
> +int
> +rte_size_to_str(char *buf, int buf_size,
> + uint64_t count, bool use_iec);
Again, no need to wrap.
> +
> /**
> * Function to terminate the application immediately, printing an error
> * message and returning the exit_code back to the shell.
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index a20c713eb1..01b6a7c190 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -398,6 +398,9 @@ EXPERIMENTAL {
> # added in 24.11
> rte_bitset_to_str;
> rte_lcore_var_alloc;
> +
> + # added in 25.07
> + rte_size_to_str;
> };
>
> INTERNAL {
> --
> 2.48.1.vfs.0.1
>
