Skip to content

Use locale-aware decimal separator in zfs_nicenum_format - #18881

Closed
mmustafasenoglu wants to merge 3 commits into
openzfs:masterfrom
mmustafasenoglu:fix/locale-decimal-separator
Closed

Use locale-aware decimal separator in zfs_nicenum_format#18881
mmustafasenoglu wants to merge 3 commits into
openzfs:masterfrom
mmustafasenoglu:fix/locale-decimal-separator

Conversation

@mmustafasenoglu

Copy link
Copy Markdown

Summary

Fixes #16987

OpenZFS command output (zfs list, zpool status, etc.) always uses the C locale decimal separator (.), ignoring the user's LC_NUMERIC locale setting. In locales that use , as the decimal separator (e.g. de_DE, fr_FR), values like 10.3G should appear as 10,3G.

This is a regression from OpenZFS 0.7.x, where locale settings were respected. The issue affects both Linux and FreeBSD (confirmed by cbinner in the issue).

Problem

The snprintf("%.*f") call in zfs_nicenum_format() at lib/libzutil/zutil_nicenum.c:142 uses the C locale's decimal point (.) by default. The C snprintf function does not respect LC_NUMERIC locale settings.

Fix

Added zfs_nicenum_locale_decimal() helper function that:

  1. Uses localeconv() to get the locale's decimal separator
  2. Replaces the first . in the formatted buffer with the locale-specific separator

This is called after the snprintf("%.*f") formatting in the else branch of zfs_nicenum_format(). The ZFS_NICENUM_TIME format is not affected since it uses integer formatting (%d).

Locale behavior

Locale Before After
C / en_US 10.3G 10.3G (no change)
de_DE 10.3G 10,3G
fr_FR 10.3G 10,3G

Changes

  • lib/libzutil/zutil_nicenum.c: Added #include <locale.h>, zfs_nicenum_locale_decimal() helper, and locale decimal replacement after snprintf

Notes

  • The fix is backward-compatible: locales using . (C, en_US, etc.) see no change
  • Screen scrapers using LANG=C are unaffected
  • The ZFS_NICENUM_TIME format (integer output) is not affected
  • zfs_isnumber() input parsing is intentionally not changed — this fix addresses output formatting only

The snprintf("%.*f") call in zfs_nicenum_format() always uses
the C locale decimal point ('.'), ignoring the user's LC_NUMERIC
locale setting. This causes commands like "zfs list" and "zpool
status" to display decimal values with '.' even in locales that
use ',' (e.g. de_DE, fr_FR).

Added zfs_nicenum_locale_decimal() helper that uses localeconv()
to get the locale's decimal separator and replaces '.' in the
formatted output. This restores the behavior that existed in
OpenZFS 0.7.x where locale settings were respected.

Fixes openzfs#16987
Copilot AI lite review requested due to automatic review settings August 2, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make zfs_nicenum_format() emit human-readable numeric values using the user’s locale decimal separator (e.g., 10,3G in de_DE) instead of always using ..

Changes:

  • Add a helper to replace . with the locale’s decimal separator after floating-point formatting.
  • Invoke the helper on the chosen “fits in 5 chars” formatted output path.
Suppressed comments (1)

lib/libzutil/zutil_nicenum.c:170

  • This call won’t change output for the primary CLI consumers because they explicitly force LC_NUMERIC to "C" (e.g., cmd/zfs/zfs_main.c:9447-9449 and cmd/zpool/zpool_main.c:14208-14210). With LC_NUMERIC=C, localeconv()->decimal_point remains '.', so zfs_nicenum_locale_decimal() becomes a no-op and the decimal separator stays ..
				if (snprintf(buf, buflen, "%.*f%s", i,
				    val, u) <= 5) {
					zfs_nicenum_locale_decimal(buf);
					break;
				}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/libzutil/zutil_nicenum.c Outdated
Comment on lines +45 to +47
/* If locale uses '.' or is empty, nothing to change */
if (dp == NULL || dp[0] == '.' || dp[0] == '\0')
return;
The snprintf("%.*f") call in zfs_nicenum_format() always uses
the C locale decimal point ('.'), ignoring the user's LC_NUMERIC
locale setting. This causes commands like "zfs list" and "zpool
status" to display decimal values with '.' even in locales that
use ',' (e.g. de_DE, fr_FR).

Added zfs_nicenum_locale_decimal() helper that uses localeconv()
to get the locale's decimal separator and replaces '.' in the
formatted output. This restores the behavior that existed in
OpenZFS 0.7.x where locale settings were respected.

Signed-off-by: Mustafa Senoglu <mmustafasenoglu0@gmail.com>

Fixes openzfs#16987
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 4, 2026
@mmustafasenoglu

Copy link
Copy Markdown
Author

Good catch on the multibyte decimal_point concern. Youre right that in locales like Arabic (U+066B, 2 UTF-8 bytes), dp[0] would only grab the first byte and corrupt the output.

However, in practice the ZFS CLI forces LC_NUMERIC="C" before calling zfs_nicenum_format() (see cmd/zfs/zfs_main.c:9447 and cmd/zpool/zpool_main.c:14208), which means localeconv()->decimal_point is always "." in the actual code paths that reach this helper. So dp[0] == . short-circuits before any replacement happens.

That said, if someone calls zfs_nicenum_format() directly without forcing LC_NUMERIC=C, the multibyte corruption would be real. The safer approach would be to check strlen(dp) > 1 and skip the replacement entirely when the separator is multibyte:

if (dp == NULL || dp[0] == \0 || strlen(dp) > 1)
    return;

This way we gracefully skip locales where we cant safely do a single-byte replacement. Ill add this guard in the next push.

Arabic and other locales use multibyte decimal separators (e.g.
U+066B). Since zfs_nicenum_locale_decimal() replaces only a single
byte, this would corrupt the output. Add a strlen(dp) > 1 guard
to skip locales where the separator doesn't fit in one byte.

In practice, the ZFS CLI forces LC_NUMERIC="C" before calling
zfs_nicenum_format(), so this is a defensive measure for direct
callers who don't set the locale.
Copilot AI review requested due to automatic review settings August 6, 2026 08:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@behlendorf

Copy link
Copy Markdown
Contributor

There's a bit of history here. Commit c2c7ca0 explicitly set "C" as the LC_NUMERIC to force the decimal separator. At the time the notion was it was preferable to standardize the CLI across locales to facilitate scripting. That's led to some confusion, and it's a decision we should probably revisit. Let's move this discussion over to PR #17075 where there were some good suggestions.

@behlendorf behlendorf closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZFS command output uses wrong comma separator

3 participants