Pádraig Brady <[email protected]> writes: > That's abit of a layering violation, as it assumes > umaxtostr() will always write to the end of the buffer. > This might be a place to open code umaxtostr() like was done in: > https://github.com/coreutils/coreutils/commit/0619c4a49
Ah, good point. I thought it was safe to assume that wouldn't change. I didn't know about that commit. I pushed the v2 patch hand rolling inttostr there. Also, I added a test that will catch any silly typos. E.g., using too small of a type for INT_BUFSIZE_BOUND. Collin
>From ebe2b7513e36fc3be99c9c57b0bc2c136e387fae Mon Sep 17 00:00:00 2001 Message-ID: <ebe2b7513e36fc3be99c9c57b0bc2c136e387fae.1778222803.git.collin.fu...@gmail.com> From: Collin Funk <[email protected]> Date: Wed, 6 May 2026 20:39:20 -0700 Subject: [PATCH v2] shuf: prefer fwrite over fputs and fputc On an AMD Ryzen 7 3700X running GNU/Linux: $ timeout 30 taskset 1 ./src/shuf-prev \ -r -i 1000000-1000000 | pv -r > /dev/null [ 302MiB/s] $ timeout 30 taskset 1 ./src/shuf \ -r -i 1000000-1000000 | pv -r > /dev/null [ 434MiB/s] * src/shuf.c (print_number): New function. (write_permuted_numbers, write_random_numbers): Use it. * tests/shuf/shuf.sh: Add a test case to run 'shuf -i' with varying numbers of digits to check that the string conversion is correct. --- src/shuf.c | 23 +++++++++++++++++------ tests/shuf/shuf.sh | 12 ++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/shuf.c b/src/shuf.c index 948ee88f3..b1c645caf 100644 --- a/src/shuf.c +++ b/src/shuf.c @@ -316,6 +316,21 @@ write_permuted_lines (size_t n_lines, char *const *line, return 0; } +/* Print NUMBER followed by EOLBYTE to standard output. + Return false on failure, true on success. */ +static bool +print_number (unsigned long int number, char eolbyte) +{ + char buf[INT_BUFSIZE_BOUND (unsigned long int)]; + char *p = buf + INT_STRLEN_BOUND (unsigned long int); + *p = eolbyte; + do + *--p = '0' + number % 10; + while ((number /= 10) != 0); + idx_t len = buf + sizeof buf - p; + return fwrite (p, 1, len, stdout) == len; +} + /* Output N_LINES of numbers to stdout, from PERMUTATION array. PERMUTATION must have at least N_LINES elements. */ static int @@ -325,9 +340,7 @@ write_permuted_numbers (size_t n_lines, size_t lo_input, for (size_t i = 0; i < n_lines; i++) { unsigned long int n = lo_input + permutation[i]; - char buf[INT_BUFSIZE_BOUND (uintmax_t)]; - if (fputs (umaxtostr (n, buf), stdout) < 0 - || fputc (eolbyte, stdout) < 0) + if (! print_number (n, eolbyte)) return -1; } @@ -345,9 +358,7 @@ write_random_numbers (struct randint_source *s, size_t count, for (size_t i = 0; i < count; i++) { unsigned long int j = lo_input + randint_choose (s, range); - char buf[INT_BUFSIZE_BOUND (uintmax_t)]; - if (fputs (umaxtostr (j, buf), stdout) < 0 - || fputc (eolbyte, stdout) < 0) + if (! print_number (j, eolbyte)) return -1; } diff --git a/tests/shuf/shuf.sh b/tests/shuf/shuf.sh index 346d9c956..387bbb649 100755 --- a/tests/shuf/shuf.sh +++ b/tests/shuf/shuf.sh @@ -100,6 +100,18 @@ shuf -n10 -i0-9 -n3 -n20 > exp || framework_failure_ c=$(wc -l < exp) || framework_failure_ test "$c" -eq 3 || { fail=1; echo "Multiple -n failed">&2 ; } +# Test that the conversion from integer to string doesn't write past a buffer. +# Note that the value is too large for shell arithmetic. +v=$ULONG_MAX +while :; do + v=$(echo $v | sed 's/^0/1/') + test -z "$v" && break + echo $v > exp + shuf -i $v-$v > out || fail=1 + compare exp out || fail=1 + v=$(echo $v | cut -b2-) +done + # Test error conditions # -i and -e must not be used together -- 2.54.0
