Skip to content

sed: separate lines with NUL when -z is given - #585

Open
sap1110 wants to merge 2 commits into
uutils:mainfrom
sap1110:fix-zero-terminated-separator
Open

sap1110 wants to merge 2 commits into
uutils:mainfrom
sap1110:fix-zero-terminated-separator

Conversation

@sap1110

@sap1110 sap1110 commented Sep 29, 2026

Copy link
Copy Markdown

sed -z was accepted but ignored, so input was still split on newlines. This PR makes -z work as it does in GNU sed.

  • Input is split on NUL, and every output line ends with NUL.
  • The line commands (N, G, H, D, P, W, R, w, s///w, =, F, i, c) also use NUL as the separator.
  • The separator is stored in the processing context, as you suggested on the issue.
  • GNU's quirks are matched too: a text and e output keep their newline, and l shows embedded newlines as \n.
  • The M regex flag still treats newline as the line break under -z. Changing that needs work in the regex engine, so I've left it for a separate PR.
  • New unit and end-to-end tests cover the change, and each one fails without the fix.

Fixes #530

The -z (--null-data) option was accepted but ignored. Keep the line
separator in the processing context and use it to split input lines
(including those read by R), to terminate output lines (including those
of =, F, i, c, l and the w and W files), and to join lines in N, G and H
and split them in D, P and W, as GNU sed does. As in GNU sed, text
appended by a, and the output of e, are written unchanged, and l lists
a newline in the pattern space as \n.

Fixes uutils#530
@DePasqualeOrg

Copy link
Copy Markdown
Contributor

Before this PR was submitted, I already developed my own patch to fix the same issue.

I won't open it as a competing PR unless requested, but Claude Code found some cases where this PR's output differs from GNU sed 4.10 and my patch's output matches it (\0 is NUL):

  1. Input files that don't end in NUL, the usual case for text files, are joined without a separator: printf 'a' > a; printf 'b' > b; sed -z '' a b gives a\0b in GNU sed and ab with this PR.
  2. N at the end of input adds a separator to a last record that lacks one: printf 'x\ny\n' | sed -z N gives x\ny\n in GNU sed and x\ny\n\0 with this PR.
  3. The e flag of s removes the command output's trailing newline, where GNU removes a trailing NUL: printf 'x' | sed -z 's/.*/echo Y/e' gives Y\n in GNU sed and Y with this PR.
  4. When reading from a file, emptied records lose their separator: printf 'a\0b\0' > f; sed -z z f gives \0\0 in GNU sed and nothing with this PR. The same input through a pipe works.

1 and 2 are older bugs that also occur without -z, but under -z they affect most text files.

@codspeed

codspeed Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 3 benchmarks

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
❌ 3 regressed benchmarks
✅ 5 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ access_log_translit 1 s 1 s -3.31%
❌ access_log_subst 2.3 s 2.4 s -3.2%
❌ number_fix 1.2 s 1.3 s -2.12%
⚡ no_op_short 2.5 s 2.4 s +3.97%
⚡ access_log_no_op 1.7 s 1.7 s +2.83%
⚡ access_log_no_subst 1.1 s 1.1 s +2.11%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing sap1110:fix-zero-terminated-separator (05b8c9d) with main (0554c50)

Open in CodSpeed

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.77358% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.32%. Comparing base (dfae00d) to head (05b8c9d).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/sed/processor.rs 43.75% 36 Missing ⚠️
src/sed/fast_io.rs 93.57% 7 Missing ⚠️
src/sed/named_writer.rs 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #585      +/-   ##
==========================================
+ Coverage   83.67%   84.32%   +0.65%     
==========================================
  Files          14       14              
  Lines        7067     7349     +282     
  Branches      413      428      +15     
==========================================
+ Hits         5913     6197     +284     
+ Misses       1149     1146       -3     
- Partials        5        6       +1     
Flag Coverage Δ
macos_latest 85.43% <81.86%> (+0.63%) ⬆️
ubuntu_latest 85.61% <81.86%> (+0.61%) ⬆️
windows_latest 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 16 / FAILED: 41 / SKIPPED: 8
  Reference: TOTAL: 65 / PASSED: 17 / FAILED: 40 / SKIPPED: 8

Changes from main branch:
  TOTAL: +0
  PASSED: -1
  FAILED: +1

New test failures (2):
  - inplace-hold
  - stdin-prog

Test improvements (1):
  + nulldata

@github-actions

Copy link
Copy Markdown

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 16 / FAILED: 41 / SKIPPED: 8
  Reference: TOTAL: 65 / PASSED: 14 / FAILED: 43 / SKIPPED: 8

Changes from main branch:
  TOTAL: +0
  PASSED: +2
  FAILED: -2

Test improvements (2):
  + cmd-R
  + nulldata

- Keep a mmapped line's NUL terminator when the line is modified, so
  that records emptied by z keep their separator with -z.
- Strip a trailing NUL rather than \n from the output of e with -z.
- Output the separator missing at the end of one input file before the
  next file's output, and don't add one to an unterminated line left by
  N at the end of input.
- Make Q output nothing more, not even appended text or a missing
  separator, as in GNU sed.
@sap1110

sap1110 commented Oct 1, 2026

Copy link
Copy Markdown
Author

@DePasqualeOrg thanks for checking this against GNU sed. All four are fixed in 05b8c9d:

  1. Files that don't end in a separator are now separated from the next file. The stdout buffer was recreated for each input file, which lost the pending separator, so it is now kept across files. Test: test_unterminated_files_are_separated (with and without -z). This changed the multiple_input_files fixture, which had recorded the old output; it now matches GNU sed.
  2. N at end of input now keeps the last line's terminated state instead of always adding a separator. Test: test_n_at_end_keeps_missing_separator. test_uppercase_delete_prevents_automatic_printing expected the old line3\n, and now expects line3 as GNU sed gives.
  3. The e flag and command now strip a trailing NUL under -z, and a trailing \n otherwise. Test: test_null_data_s_e_flag.
  4. Converting a mmapped line to an owned one still checked for \n to decide whether it was terminated. It now uses the separator that was actually read. Test: test_null_data_file_emptied_records.

Fixing 1 exposed one more case: sed 2Q a b output the separator missing after a, which GNU sed doesn't. Q now returns without any more output, which also stops it from flushing a text, as in GNU. Test: test_quit_silently_outputs_nothing_more.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 18 / FAILED: 39 / SKIPPED: 8
  Reference: TOTAL: 65 / PASSED: 17 / FAILED: 40 / SKIPPED: 8

Changes from main branch:
  TOTAL: +0
  PASSED: +1
  FAILED: -1

Test improvements (1):
  + nulldata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adjust record separator when --zero-terminated is specified

2 participants