Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix ZFS command output uses wrong comma separator #17075

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Egor-OSSRevival
Copy link

Motivation and Context

Currently, OpenZFS incorrectly uses a full stop (.) as the decimal separator in its human-readable output, even in locales that require a comma (,). This issue not only affects aesthetics but also breaks tools like sort -h, which rely on correct numerical formatting.

For example, running:

$ zfs list -o used | sort -h

produces incorrect sorting when using a locale that expects a comma separator.

This issue does not exist in illumos' ZFS, where numeric formatting respects the system locale.

This PR ensures that OpenZFS respects the system’s locale when formatting numerical output.

Description

This change ensures that OpenZFS uses the correct decimal separator based on the system locale.

The fix involves adding:

setlocale(LC_NUMERIC, "");

to lib/libzutil/zutil_nicenum.c, ensuring that numeric output is formatted according to the user's locale settings.

How Has This Been Tested?

The changes were tested on a system with different locales, including en_US.UTF-8 and de_DE.UTF-8, to verify that the decimal separator correctly follows the system locale. The output of zfs list -o used | sort -h was checked before and after applying the fix, ensuring that numeric sorting behaves as expected. Additional testing confirmed that the change does not affect systems where the default decimal separator is a period (.).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

ZFS incorrectly uses a full stop (.) as the decimal separator in
human-readable output, even in locales that require a comma (,).
This inconsistency causes issues with tools like `sort -h`, making
sorting unreliable.

This fix ensures that ZFS respects the system locale when formatting
numerical output by setting `LC_NUMERIC` via `setlocale()`.

Signed-off-by: Egor Kovalcuk <[email protected]>
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

I am not really fresh on locales, but setting it per value printed does not sound right.

@amotin amotin linked an issue Feb 20, 2025 that may be closed by this pull request
@Egor-OSSRevival
Copy link
Author

Could you explain a bit more about what you mean and how I could do it better? Like, should I set the locale once somewhere else?

@amotin
Copy link
Member

amotin commented Feb 21, 2025

Like, should I set the locale once somewhere else?

Yea, I'd expect it to be set somewhere at the program start.

@amotin amotin added Status: Revision Needed Changes are required for the PR to be accepted Status: Code Review Needed Ready for review and testing labels Feb 22, 2025
@Egor-OSSRevival
Copy link
Author

(void) setlocale(LC_NUMERIC, "C");

Changing this line should fix issue too, but I don't know if it's good idea, maybe it's there for a reason. Could you tell me if it's okay?

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 Status: Revision Needed Changes are required for the PR to be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZFS command output uses wrong comma separator
2 participants