Skip to content

od: print -S offsets through InputOffset - #15648

Merged
sylvestre merged 1 commit into
uutils:mainfrom
ozenyunus35:od-strings-reuse-offset-format
Oct 24, 2026
Merged

sylvestre merged 1 commit into
uutils:mainfrom
ozenyunus35:od-strings-reuse-offset-format

Conversation

@ozenyunus35

Copy link
Copy Markdown
Contributor

Follow-up to #15625, as suggested in #15625 (comment).

extract_strings_from_input had its own copy of the offset formats, which is how the -S -Ax width drifted from the normal dump. It now prints the offset through InputOffset, so both use the same formats.

  • InputOffset implements Display with the formats that format_byte_offset used. format_byte_offset now returns self.to_string(), so its callers and unit tests are unchanged.
  • -S writes the offset with writeln!(writer, "{offset} {string_content}"). With -An it still prints only the string. I first used format_byte_offset() there, but the String it allocates for each string made od -S4 about 2% slower on a 400k-string file. With Display there is no extra allocation, and the time is the same as on main.

There is no change in output. All test_od tests and the uu_od unit tests pass. od -S with -Ad, -Ax, -Ao, -An, -j and -N, plus a few normal dumps, give the same output as GNU 9.12. cargo bench -p uu_od and timings of od -S4, od -Ax -S4 and od -Ax -tx1 with a release build show no change against the merge base beyond run-to-run noise.

I used an AI assistant to help draft parts of the code; I reviewed the changes and ran the tests myself.

Follow-up to uutils#15625: the strings mode had its own copy of the offset
formats. InputOffset now implements Display, format_byte_offset uses
it, and -S writes the offset through it without an extra allocation.
@github-actions

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 155.04 MB (-308 KB, -0.19%)

Significant per-binary changes:
  date     2.61 MB ->    2.30 MB  (-316 KB, -11.81%)

@sylvestre
sylvestre merged commit 294f4c7 into uutils:main Oct 24, 2026
100 checks passed
@sylvestre

Copy link
Copy Markdown
Contributor

Thanks for your PR

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.

2 participants