hash_readwrite_autotest: fix printf parameters

Compiling with MSVC logs the warnings below, which result in
build error:

../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
    unknown type field character ''' in format specifier
../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
    too many arguments passed for format string
../app/test/test_hash_readwrite.c(75): note: placeholders and
    their parameters expect 2 variadic arguments, but 4 were provided

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test/test_hash_readwrite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:
> Compiling with MSVC logs the warnings below, which result in
> build error:
> 
> ../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
>     unknown type field character ''' in format specifier
> ../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
>     too many arguments passed for format string
> ../app/test/test_hash_readwrite.c(75): note: placeholders and
>     their parameters expect 2 variadic arguments, but 4 were provided
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  app/test/test_hash_readwrite.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
IF the "'" character is not supported, is there some other method to do
thousands grouping in MSVC?

/Bruce
  

On Fri, Mar 07, 2025 at 09:01:28AM +0000, Bruce Richardson wrote:
> On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:
> > Compiling with MSVC logs the warnings below, which result in
> > build error:
> > 
> > ../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
> >     unknown type field character ''' in format specifier
> > ../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
> >     too many arguments passed for format string
> > ../app/test/test_hash_readwrite.c(75): note: placeholders and
> >     their parameters expect 2 variadic arguments, but 4 were provided
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  app/test/test_hash_readwrite.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> IF the "'" character is not supported, is there some other method to do
> thousands grouping in MSVC?
> 
> /Bruce

The problem exists with all compilers I tried on Windows:

1) MSVC logs the error I mentioned above

2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special
character. As an example,
printf("%'d\n", 1024);
results in
'd 

It seems that for this syntax to work as you would expect, support needs to exist in both the
compiler and the libraries used.

Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping.
If really needed (and I understand it is useful for large numbers), we could get the same result
by calling a helper function that would convert the number in the formatted string and use that
in the printf statement.

There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
(https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)

We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).

Let me know if this is something that would need to be done, or if the current output
without thousands grouping is good enough.
--
Andre Muezerie
  

On Fri, Mar 07, 2025 at 02:34:01PM -0800, Andre Muezerie wrote:
> On Fri, Mar 07, 2025 at 09:01:28AM +0000, Bruce Richardson wrote:
> > On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:
> > > Compiling with MSVC logs the warnings below, which result in
> > > build error:
> > > 
> > > ../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
> > >     unknown type field character ''' in format specifier
> > > ../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
> > >     too many arguments passed for format string
> > > ../app/test/test_hash_readwrite.c(75): note: placeholders and
> > >     their parameters expect 2 variadic arguments, but 4 were provided
> > > 
> > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > ---
> > >  app/test/test_hash_readwrite.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > IF the "'" character is not supported, is there some other method to do
> > thousands grouping in MSVC?
> > 
> > /Bruce
> 
> The problem exists with all compilers I tried on Windows:
> 
> 1) MSVC logs the error I mentioned above
> 
> 2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special
> character. As an example,
> printf("%'d\n", 1024);
> results in
> 'd 
> 
> It seems that for this syntax to work as you would expect, support needs to exist in both the
> compiler and the libraries used.
> 
> Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping.
> If really needed (and I understand it is useful for large numbers), we could get the same result
> by calling a helper function that would convert the number in the formatted string and use that
> in the printf statement.
> 
> There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
> (https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)
> 
> We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).
> 
> Let me know if this is something that would need to be done, or if the current output
> without thousands grouping is good enough.
> --
The thousands grouping is incredibly helpful when working with large
numbers, but given the lack of support for this on Windows, we'll just have
to go without, I think.

/Bruce
  

On Mon, 10 Mar 2025 10:51:51 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Fri, Mar 07, 2025 at 02:34:01PM -0800, Andre Muezerie wrote:
> > On Fri, Mar 07, 2025 at 09:01:28AM +0000, Bruce Richardson wrote:  
> > > On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:  
> > > > Compiling with MSVC logs the warnings below, which result in
> > > > build error:
> > > > 
> > > > ../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
> > > >     unknown type field character ''' in format specifier
> > > > ../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
> > > >     too many arguments passed for format string
> > > > ../app/test/test_hash_readwrite.c(75): note: placeholders and
> > > >     their parameters expect 2 variadic arguments, but 4 were provided
> > > > 
> > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > ---
> > > >  app/test/test_hash_readwrite.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >   
> > > IF the "'" character is not supported, is there some other method to do
> > > thousands grouping in MSVC?
> > > 
> > > /Bruce  
> > 
> > The problem exists with all compilers I tried on Windows:
> > 
> > 1) MSVC logs the error I mentioned above
> > 
> > 2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special
> > character. As an example,
> > printf("%'d\n", 1024);
> > results in
> > 'd 
> > 
> > It seems that for this syntax to work as you would expect, support needs to exist in both the
> > compiler and the libraries used.
> > 
> > Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping.
> > If really needed (and I understand it is useful for large numbers), we could get the same result
> > by calling a helper function that would convert the number in the formatted string and use that
> > in the printf statement.
> > 
> > There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
> > (https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)
> > 
> > We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).
> > 
> > Let me know if this is something that would need to be done, or if the current output
> > without thousands grouping is good enough.
> > --  
> The thousands grouping is incredibly helpful when working with large
> numbers, but given the lack of support for this on Windows, we'll just have
> to go without, I think.
> 
> /Bruce

Maybe some variation of the pretty printing code that iproute2 has
(see print_num) would be useful. Feel free to reuse it.
I wrote the initial version.
  

On Mon, Mar 10, 2025 at 08:36:40AM -0700, Stephen Hemminger wrote:
> On Mon, 10 Mar 2025 10:51:51 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Fri, Mar 07, 2025 at 02:34:01PM -0800, Andre Muezerie wrote:
> > > On Fri, Mar 07, 2025 at 09:01:28AM +0000, Bruce Richardson wrote:  
> > > > On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:  
> > > > > Compiling with MSVC logs the warnings below, which result in
> > > > > build error:
> > > > > 
> > > > > ../app/test/test_hash_readwrite.c(73): warning C4476: 'printf' :
> > > > >     unknown type field character ''' in format specifier
> > > > > ../app/test/test_hash_readwrite.c(75): warning C4474: 'printf' :
> > > > >     too many arguments passed for format string
> > > > > ../app/test/test_hash_readwrite.c(75): note: placeholders and
> > > > >     their parameters expect 2 variadic arguments, but 4 were provided
> > > > > 
> > > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > > ---
> > > > >  app/test/test_hash_readwrite.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >   
> > > > IF the "'" character is not supported, is there some other method to do
> > > > thousands grouping in MSVC?
> > > > 
> > > > /Bruce  
> > > 
> > > The problem exists with all compilers I tried on Windows:
> > > 
> > > 1) MSVC logs the error I mentioned above
> > > 
> > > 2) GCC and Clang don't complain at compile time, but don't honor the "'" as a special
> > > character. As an example,
> > > printf("%'d\n", 1024);
> > > results in
> > > 'd 
> > > 
> > > It seems that for this syntax to work as you would expect, support needs to exist in both the
> > > compiler and the libraries used.
> > > 
> > > Back to your question: there's no equivalent syntax on Windows that provides the thousands grouping.
> > > If really needed (and I understand it is useful for large numbers), we could get the same result
> > > by calling a helper function that would convert the number in the formatted string and use that
> > > in the printf statement.
> > > 
> > > There is a Win32 API that does that. It takes a string as input though: GetNumberFormatA.
> > > (https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getnumberformata)
> > > 
> > > We could use ifdefs to keep the old logic on Linux and use new logic on Windows (for all compilers).
> > > 
> > > Let me know if this is something that would need to be done, or if the current output
> > > without thousands grouping is good enough.
> > > --  
> > The thousands grouping is incredibly helpful when working with large
> > numbers, but given the lack of support for this on Windows, we'll just have
> > to go without, I think.
> > 
> > /Bruce
> 
> Maybe some variation of the pretty printing code that iproute2 has
> (see print_num) would be useful. Feel free to reuse it.
> I wrote the initial version.

That's an interesting suggestion. I'll use that.
  

> From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> Sent: Tuesday, 11 March 2025 15.40
> 
> On Mon, Mar 10, 2025 at 08:36:40AM -0700, Stephen Hemminger wrote:
> > On Mon, 10 Mar 2025 10:51:51 +0000
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >
> > > On Fri, Mar 07, 2025 at 02:34:01PM -0800, Andre Muezerie wrote:
> > > > On Fri, Mar 07, 2025 at 09:01:28AM +0000, Bruce Richardson wrote:
> > > > > On Thu, Mar 06, 2025 at 12:03:28PM -0800, Andre Muezerie wrote:
> > > > > > Compiling with MSVC logs the warnings below, which result in
> > > > > > build error:
> > > > > >
> > > > > > ../app/test/test_hash_readwrite.c(73): warning C4476:
> 'printf' :
> > > > > >     unknown type field character ''' in format specifier
> > > > > > ../app/test/test_hash_readwrite.c(75): warning C4474:
> 'printf' :
> > > > > >     too many arguments passed for format string
> > > > > > ../app/test/test_hash_readwrite.c(75): note: placeholders and
> > > > > >     their parameters expect 2 variadic arguments, but 4 were
> provided
> > > > > >
> > > > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > > > ---
> > > > > >  app/test/test_hash_readwrite.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > IF the "'" character is not supported, is there some other
> method to do
> > > > > thousands grouping in MSVC?
> > > > >
> > > > > /Bruce
> > > >
> > > > The problem exists with all compilers I tried on Windows:
> > > >
> > > > 1) MSVC logs the error I mentioned above
> > > >
> > > > 2) GCC and Clang don't complain at compile time, but don't honor
> the "'" as a special
> > > > character. As an example,
> > > > printf("%'d\n", 1024);
> > > > results in
> > > > 'd
> > > >
> > > > It seems that for this syntax to work as you would expect,
> support needs to exist in both the
> > > > compiler and the libraries used.
> > > >
> > > > Back to your question: there's no equivalent syntax on Windows
> that provides the thousands grouping.
> > > > If really needed (and I understand it is useful for large
> numbers), we could get the same result
> > > > by calling a helper function that would convert the number in the
> formatted string and use that
> > > > in the printf statement.
> > > >
> > > > There is a Win32 API that does that. It takes a string as input
> though: GetNumberFormatA.
> > > > (https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-
> winnls-getnumberformata)
> > > >
> > > > We could use ifdefs to keep the old logic on Linux and use new
> logic on Windows (for all compilers).
> > > >
> > > > Let me know if this is something that would need to be done, or
> if the current output
> > > > without thousands grouping is good enough.
> > > > --
> > > The thousands grouping is incredibly helpful when working with
> large
> > > numbers, but given the lack of support for this on Windows, we'll
> just have
> > > to go without, I think.
> > >
> > > /Bruce
> >
> > Maybe some variation of the pretty printing code that iproute2 has
> > (see print_num) would be useful. Feel free to reuse it.
> > I wrote the initial version.
> 
> That's an interesting suggestion. I'll use that.

Often "123 M" is better than "123123123", but sometimes seeing the last digit move is important, e.g. when looking at packet counters.

Just mentioning, so you consider which format is better every time you use it.
  

On Thu, Mar 13, 2025 at 09:51:43AM -0700, Andre Muezerie wrote:
> v4:
>  - Added parameter "unit", which allows rte_size_to_str() to be smarter
>    about the need to append a space after the number. This keeps the
>    function easy to use and avoids the need for complex explanations
>    about the space that could be needed between number and multiple/unit.
>  - Changed the return type of rte_size_to_str() to be buf, or NULL on
>    error, allowing the function to be inlined.
>  - Removed a line wrap.
> 
>  v3:
>  - Added rte_size_to_str to version.map (marked experimental for 25.07) so
>    that symbol gets exported when building with -Ddefault_library=shared
>  - Added sample outputs
>  - Added a space between the number and the postfix (if a postfix is
>    present)
> 
> Andre Muezerie (3):
>   eal: add function rte_size_to_str
>   hash_multiwriter_autotest: fix printf parameters
>   hash_readwrite_autotest: fix printf parameters
> 
Series-Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

On Wed, Apr 9, 2025 at 3:21 AM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> v5:
>  - Rebased and updated to conform to new function versioning mechanism.
>
> v4:
>  - Added parameter "unit", which allows rte_size_to_str() to be smarter
>    about the need to append a space after the number. This keeps the
>    function easy to use and avoids the need for complex explanations
>    about the space that could be needed between number and multiple/unit.
>  - Changed the return type of rte_size_to_str() to be buf, or NULL on
>    error, allowing the function to be inlined.
>  - Removed a line wrap.
>
>  v3:
>  - Added rte_size_to_str to version.map (marked experimental for 25.07) so
>    that symbol gets exported when building with -Ddefault_library=shared
>  - Added sample outputs
>  - Added a space between the number and the postfix (if a postfix is
>    present)
>
> Andre Muezerie (3):
>   eal: add function rte_size_to_str
>   hash_multiwriter_autotest: fix printf parameters
>   hash_readwrite_autotest: fix printf parameters
>
>  app/test/test_hash_multiwriter.c       | 14 +++++--
>  app/test/test_hash_readwrite.c         | 14 ++++++-
>  lib/eal/common/eal_common_string_fns.c | 54 ++++++++++++++++++++++++++
>  lib/eal/include/rte_common.h           | 34 ++++++++++++++++
>  4 files changed, 111 insertions(+), 5 deletions(-)

It looks odd that string formatting helpers are declared in rte_common.h.
I would move those declarations to rte_string_fns.h but this can wait
a followup patch since other helper was already in rte_common.h.


Series applied, thanks.