Re: [PATCH] Add support for printf formatting with non-monetary grouping chars

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

I would update the commit summary (one liner) to state that this patch fixes
the non-monetary goruping rather than adding support.  The code was there to
implement it - the only thing we missed was the setlocale() call to ensure
that setting the locale with an env var would actually have an effect.

On Wed, Jan 14, 2026 at 12:09:36AM -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.
> 
> Add such support.
> 

This is also a bit misleading I think.  Not only does DTrace cocument the
support - the code actually supports it.  Just the setlocale() call was
missing.

I think the commit message should point that out rather than potentially giving
the impression that this patch implements it.

> Orabug: 30430270
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  cmd/dtrace.c                         |  5 ++++-
>  test/unittest/printf/tst.grouping.r  |  5 +++++
>  test/unittest/printf/tst.grouping.sh | 29 ++++++++++++++++++++++++++++
>  test/unittest/printf/tst.grouping.x  | 13 +++++++++++++
>  4 files changed, 51 insertions(+), 1 deletion(-)
>  create mode 100644 test/unittest/printf/tst.grouping.r
>  create mode 100755 test/unittest/printf/tst.grouping.sh
>  create mode 100755 test/unittest/printf/tst.grouping.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.r b/test/unittest/printf/tst.grouping.r
> new file mode 100644
> index 000000000..077a0b7f9
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping.r
> @@ -0,0 +1,5 @@
> +123456789
> +
> +123,456,789
> +
> +success
> diff --git a/test/unittest/printf/tst.grouping.sh b/test/unittest/printf/tst.grouping.sh
> new file mode 100755
> index 000000000..13b953758
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping.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

Shouldn't you test both LC_ALL and LC_NUMERIC, especially since many sources
seem to suggest that setting LC_ALL is a bad idea, and that people should set
the specific sub-areas they need (since LC_ALL is an override for all and
might have other effects people do not intend).

> +
> +	$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.x b/test/unittest/printf/tst.grouping.x
> new file mode 100755
> index 000000000..9993fbcd3
> --- /dev/null
> +++ b/test/unittest/printf/tst.grouping.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
>