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

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

On Tue, Mar 11, 2025 at 08:49:05AM -0700, Stephen Hemminger wrote:
> On Tue, 11 Mar 2025 08:33:13 -0700
> Andre Muezerie <andremue@linux.microsoft.com> 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           | 21 ++++++++++++
> >  2 files changed, 65 insertions(+)
> > 
> > diff --git a/lib/eal/common/eal_common_string_fns.c b/lib/eal/common/eal_common_string_fns.c
> > index 9ca2045b18..b658d68eac 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;
> >  }
> > +
> 
> Is this right? Looks like existing code is not using correct multiple
>  
> The standard for communication is K = 1000 and the standard for storage is K = 1024.
> That is why iproute2 has the use_iec flag.
> 
> https://en.wikipedia.org/wiki/Data-rate_units
>

Ideally, we should have that flag, but I believe this current function is
for things like memory sizes, which means that using 1024 is correct.

/Bruce