Re: [PATCH] Fix printf formatting with non-monetary grouping chars

[Kris Van Hees <kris.van.hees@oracle.com>]

One small change I overlooked in my previous review.  See at end of email...

On Wed, Jan 14, 2026 at 04:10:18PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Non-monetary grouping characters are used to separate groups of digits
> in numbers that are not monetary values.  DTrace used to document
> support for printf formatting, using a single quote, for such grouping.
> Currently, support is largely implemented, but simply not working.
> 
> Add a setlocale() call to pick LC_NUMERIC up from environment variables.
> 
> Orabug: 30430270
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  cmd/dtrace.c                                  |  5 +++-
>  test/unittest/printf/tst.grouping-LC_ALL.r    |  5 ++++
>  test/unittest/printf/tst.grouping-LC_ALL.sh   | 29 +++++++++++++++++++
>  test/unittest/printf/tst.grouping-LC_ALL.x    | 13 +++++++++
>  .../unittest/printf/tst.grouping-LC_NUMERIC.r |  5 ++++
>  .../printf/tst.grouping-LC_NUMERIC.sh         | 29 +++++++++++++++++++
>  .../unittest/printf/tst.grouping-LC_NUMERIC.x | 13 +++++++++
>  7 files changed, 98 insertions(+), 1 deletion(-)
>  create mode 100644 test/unittest/printf/tst.grouping-LC_ALL.r
>  create mode 100755 test/unittest/printf/tst.grouping-LC_ALL.sh
>  create mode 100755 test/unittest/printf/tst.grouping-LC_ALL.x
>  create mode 100644 test/unittest/printf/tst.grouping-LC_NUMERIC.r
>  create mode 100755 test/unittest/printf/tst.grouping-LC_NUMERIC.sh
>  create mode 100755 test/unittest/printf/tst.grouping-LC_NUMERIC.x
> 
> diff --git a/cmd/dtrace.c b/cmd/dtrace.c
> index baa73e9d7..6cd7aeadb 100644
> --- a/cmd/dtrace.c
> +++ b/cmd/dtrace.c
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2025, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2026, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -9,6 +9,7 @@
>  #include <sys/stat.h>
>  #include <sys/wait.h>
>  #include <sys/compiler.h>
> +#include <locale.h>
>  
>  #include <dtrace.h>
>  #include <stdlib.h>
> @@ -924,6 +925,8 @@ main(int argc, char *argv[])
>  	pid_t pid;
>  	struct dtrace_proc *proc;
>  
> +	setlocale(LC_NUMERIC, "");
> +
>  	g_ofp = stdout;
>  
>  	g_pname = basename(argv[0]);
> diff --git a/test/unittest/printf/tst.grouping-LC_ALL.r b/test/unittest/printf/tst.grouping-LC_ALL.r
> new file mode 100644
> index 000000000..077a0b7f9
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_ALL.r
> @@ -0,0 +1,5 @@
> +123456789
> +
> +123,456,789
> +
> +success
> diff --git a/test/unittest/printf/tst.grouping-LC_ALL.sh b/test/unittest/printf/tst.grouping-LC_ALL.sh
> new file mode 100755
> index 000000000..13b953758
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_ALL.sh
> @@ -0,0 +1,29 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +
> +dtrace=$1
> +
> +for mylocale in C en_US.UTF-8; do
> +	export LC_ALL=$mylocale
> +
> +	$dtrace $dt_flags -qs /dev/stdin << EOF
> +	BEGIN
> +	{
> +		printf("%'d\n", 123456789);
> +		exit(0);
> +	}
> +EOF
> +	if [ $? -ne 0 ]; then
> +		echo ERROR: dtrace
> +		locale
> +		exit 1
> +	fi
> +done
> +
> +echo success
> +exit 0
> diff --git a/test/unittest/printf/tst.grouping-LC_ALL.x b/test/unittest/printf/tst.grouping-LC_ALL.x
> new file mode 100755
> index 000000000..9993fbcd3
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_ALL.x
> @@ -0,0 +1,13 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +
> +if locale -a | grep en_US.utf8 ; then
> +	exit 0
> +fi
> +
> +echo install glibc-langpack-en yum package for this test
> +exit 2
> diff --git a/test/unittest/printf/tst.grouping-LC_NUMERIC.r b/test/unittest/printf/tst.grouping-LC_NUMERIC.r
> new file mode 100644
> index 000000000..077a0b7f9
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_NUMERIC.r
> @@ -0,0 +1,5 @@
> +123456789
> +
> +123,456,789
> +
> +success
> diff --git a/test/unittest/printf/tst.grouping-LC_NUMERIC.sh b/test/unittest/printf/tst.grouping-LC_NUMERIC.sh
> new file mode 100755
> index 000000000..656fd83c1
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_NUMERIC.sh
> @@ -0,0 +1,29 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +
> +dtrace=$1
> +
> +for mylocale in C en_US.UTF-8; do
> +	export LC_NUMERIC=$mylocale
> +
> +	$dtrace $dt_flags -qs /dev/stdin << EOF
> +	BEGIN
> +	{
> +		printf("%'d\n", 123456789);
> +		exit(0);
> +	}
> +EOF
> +	if [ $? -ne 0 ]; then
> +		echo ERROR: dtrace
> +		locale
> +		exit 1
> +	fi
> +done
> +
> +echo success
> +exit 0
> diff --git a/test/unittest/printf/tst.grouping-LC_NUMERIC.x b/test/unittest/printf/tst.grouping-LC_NUMERIC.x
> new file mode 100755
> index 000000000..9993fbcd3
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping-LC_NUMERIC.x
> @@ -0,0 +1,13 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +
> +if locale -a | grep en_US.utf8 ; then
> +	exit 0
> +fi
> +
> +echo install glibc-langpack-en yum package for this test
> +exit 2
> -- 
> 2.47.3
> 

Here and in tst.grouping-LC_ALL.x, mentioning a reference to a specific package
is not generally helpful because other distros do not use yum and the locale
data is provided by other packages.  Perhaps just mention that the en.US.utf8
locale cannot be found?

Also, you test for en_US.utf8 here but set LC_ALL and LC_NUMERIC to en_US.UTF-8.
Perhaps one or the other should be used consistently?