From 10c5c15ff3d2b4dad2eed6a3413e6fc616db1f19 Mon Sep 17 00:00:00 2001 From: Socialpranker <0630039863abc@gmail.com> Date: Mon, 31 Aug 2026 14:38:03 +0200 Subject: [PATCH] sort: apply -n to the key, not the whole line `-n` parses a number from every line up front and compares those numbers before any key is looked at. That shortcut is only the same comparison when the key spans the whole line, but nothing checked that: printf "19\\n21\\n" | sort -n -k1.2 sorts on "9" and "1", so GNU prints 21 first; uutils compared 19 against 21 and printed 19. The same went for a key picked out by field (`-n -t. -k2`), and for a key with its own `r`, which the shortcut could not honor because it only knows the global ordering. Guard the shortcut with `can_use_whole_line_numeric`, in the shape the neighboring `can_use_fast_lexicographic` already uses: a single key, numeric, not reversed, starting at the first character of the first field and running to the end of the line. Plain `-n` and `-n -k1` still take it. --- src/uu/sort/src/chunks.rs | 6 ++--- src/uu/sort/src/sort.rs | 26 ++++++++++++++++++-- tests/by-util/test_sort.rs | 50 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index 3387817b06a..ca9efcd13e8 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -20,9 +20,7 @@ use self_cell::self_cell; use uucore::error::{UResult, USimpleError, strip_errno}; use uucore::translate; -use crate::{ - GeneralBigDecimalParseResult, GlobalSettings, Line, SortMode, numeric_str_cmp::NumInfo, -}; +use crate::{GeneralBigDecimalParseResult, GlobalSettings, Line, numeric_str_cmp::NumInfo}; const ALLOC_CHUNK_SIZE: usize = 64 * 1024; const MAX_TOKEN_BUFFER_BYTES: usize = 4 * 1024 * 1024; @@ -312,7 +310,7 @@ fn parse_lines<'a>( .parsed_floats .reserve(estimated.saturating_mul(settings.precomputed.floats_per_line)); } - if settings.mode == SortMode::Numeric { + if settings.precomputed.whole_line_numeric { line_data.line_num_floats.reserve(estimated); } let mut start = 0usize; diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 7a8bd550fdb..f47f457eda9 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -332,6 +332,7 @@ struct Precomputed { fast_lexicographic: bool, fast_locale_collation: bool, fast_ascii_insensitive: bool, + whole_line_numeric: bool, tokenize_blank_thousands_sep: bool, tokenize_allow_unit_after_blank: bool, } @@ -397,6 +398,27 @@ impl GlobalSettings { self.precomputed.fast_locale_collation = disable_fast_lexicographic && self.can_use_fast_lexicographic(); self.precomputed.fast_ascii_insensitive = self.can_use_fast_ascii_insensitive(); + self.precomputed.whole_line_numeric = self.can_use_whole_line_numeric(); + } + + /// Returns true when a number parsed from the whole line can stand in for + /// the key. + /// + /// `-n` parses the line once up front and compares those numbers before + /// looking at any key. That is only the same comparison when the single + /// key spans the entire line: `-n -k1.2` sorts on the line's second + /// character onwards, and `-n -t. -k2` on its second field, neither of + /// which the line as a whole stands for. A key of its own `r` also has to + /// go the long way, since the shortcut only knows the global one. + fn can_use_whole_line_numeric(&self) -> bool { + self.mode == SortMode::Numeric && self.selectors.len() == 1 && { + let selector = &self.selectors[0]; + selector.settings.mode == SortMode::Numeric + && !selector.settings.reverse + && selector.from.field == 1 + && selector.from.char == 1 + && selector.to.is_none() + } } /// Returns true when the fast lexicographic path can be used safely. @@ -654,7 +676,7 @@ impl<'a> Line<'a> { || settings.precomputed.selections_per_line > 0 || settings.precomputed.num_infos_per_line > 0 || settings.precomputed.floats_per_line > 0 - || settings.mode == SortMode::Numeric; + || settings.precomputed.whole_line_numeric; if !needs_line_data { return Self { line, index }; } @@ -667,7 +689,7 @@ impl<'a> Line<'a> { &settings.precomputed, ); } - if settings.mode == SortMode::Numeric { + if settings.precomputed.whole_line_numeric { // exclude inf, nan, scientific notation; GNU -n does not treat '+' as a sign let line_num_float = (!line.iter().any(u8::is_ascii_alphabetic)) .then(|| std::str::from_utf8(line).ok()) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 0747ddc3935..44c59b90597 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -325,6 +325,56 @@ fn test_numeric_sort_rejects_leading_plus_sign() { .stdout_is("+1\n+10\n+2\n"); } +#[test] +fn test_numeric_sort_uses_the_key_not_the_whole_line() { + // `-n` compares a number parsed from the line as a whole, which only + // stands for the key when the key is the whole line. + + // Character offset: the key is "9" and "1", so 21 comes first. Comparing + // the lines instead puts 19 first. + new_ucmd!() + .args(&["-n", "-k1.2"]) + .pipe_in("19\n21\n") + .succeeds() + .stdout_is("21\n19\n"); + + // Field: with '.' as the separator the key is "9" and "1" again, while + // both lines parse as numbers on their own. + new_ucmd!() + .args(&["-n", "-t.", "-k2"]) + .pipe_in("1.9\n2.1\n") + .succeeds() + .stdout_is("2.1\n1.9\n"); + + // A key that reverses on its own: the shortcut only knows the global + // ordering, so it has to go the long way. + new_ucmd!() + .args(&["-n", "-k1r"]) + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n21\n19\n"); + + // Equal keys fall back to comparing the lines byte by byte, not + // numerically. + new_ucmd!() + .args(&["-n", "-k1.2"]) + .pipe_in("1\n10\n2\n20\n3\n") + .succeeds() + .stdout_is("1\n10\n2\n20\n3\n"); + + // The whole-line cases keep working. + new_ucmd!() + .arg("-n") + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n19\n21\n"); + new_ucmd!() + .args(&["-n", "-k1"]) + .pipe_in("19\n21\n3\n") + .succeeds() + .stdout_is("3\n19\n21\n"); +} + #[test] fn test_check_zero_terminated_failure() { new_ucmd!()