Skip to content

Fix hex revert and little-endian padding bugs - #36

Closed
xyproto wants to merge 3 commits into
mainfrom
fix-hex-revert-and-le-padding-bugs-4771549755149731832
Closed

xyproto wants to merge 3 commits into
mainfrom
fix-hex-revert-and-le-padding-bugs-4771549755149731832

Conversation

@xyproto

@xyproto xyproto commented Mar 29, 2026

Copy link
Copy Markdown
Owner

I have fixed two major bugs and performed some code cleanup in main.c.

  1. CRLF Hex Revert Bug: The hex revert logic was incorrectly stopping whenever it encountered a \r character. This meant that on Windows-style hex dumps (with CRLF), only the first line would be processed. I removed the && c != '\r' condition from the while loops in decode_hex_stream_normal and decode_hex_stream_bits to allow processing to continue.

  2. Little-Endian Padding Bug: In little-endian mode (-e), the hex area was not being properly padded with spaces when the last line was partial and color was disabled. This caused the ASCII column to be misaligned. I added a loop to correctly pad the hex area, using the appropriate XOR-based byte order calculation. I also fixed an issue where the newline was incorrectly overwriting ASCII characters in some cases due to improper max_idx updates.

  3. Code Cleanup: I removed the redundant int e parameter from several functions (hex_postscript, hex_cinclude, etc.) because it was being overwritten by a local getc_or_die call anyway.

All changes have been verified with make test and new reproduction test cases.


PR created automatically by Jules for task 4771549755149731832 started by @xyproto

- Fix a bug where hex revert (`-r`) would stop prematurely on files with CRLF line endings.
- Fix missing padding in the hex area for little-endian output (`-e`) on partial last lines.
- Remove redundant `int e` argument from several hex output functions as it was immediately overwritten by `getc_or_die`.
- Correct `max_idx` calculation in `hex_littleendian` to prevent the newline from overwriting ASCII characters.

Co-authored-by: xyproto <52813+xyproto@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@xyproto

xyproto commented Mar 30, 2026

Copy link
Copy Markdown
Owner Author

I don't think this is trustworthy. Will need to add tests for this first.

@google-labs-jules

Copy link
Copy Markdown

I don't think this is trustworthy. Will need to add tests for this first.

I've added formal tests to the Makefile (test_revert_crlf and test_le_padding) to verify these fixes. These tests are now part of the make test suite and pass successfully.

- Added `test_revert_crlf` to verify that hex revert handles CRLF line endings.
- Added `test_le_padding` to verify that little-endian output correctly pads partial lines.
- Included these new tests in the main `test` target.

Co-authored-by: xyproto <52813+xyproto@users.noreply.github.com>
@xyproto

xyproto commented Mar 31, 2026

Copy link
Copy Markdown
Owner Author

Removing the && c != '\r' check means that tinyxxd would no longer be a drop-in replacement for xxd?

@google-labs-jules

Copy link
Copy Markdown

Removing the && c != '\r' check means that tinyxxd would no longer be a drop-in replacement for xxd?

Actually, removing this check makes tinyxxd a better drop-in replacement. I've verified that the reference xxd implementation correctly handles CRLF line endings in revert mode and processes the entire file. Before this change, tinyxxd would stop at the first \r, which is inconsistent with xxd's behavior.

- Added formal regression tests to the Makefile.
- Verified that removing the CRLF check aligns better with reference xxd behavior.

Co-authored-by: xyproto <52813+xyproto@users.noreply.github.com>
@xyproto xyproto closed this Apr 1, 2026
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.

1 participant