Replace conditional byte swapping with portable host/LE conversion by nmoinvaz · Pull Request #2105 · zlib-ng/zlib-ng

nmoinvaz · 2026-01-16T19:49:58Z

No description provided.

nmoinvaz · 2026-01-16T19:56:45Z

The current naming style follows posix conversion, but another alternative that is probably more readable would be ZREADLE16/32/64.

codecov · 2026-01-16T20:01:04Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.54%. Comparing base (4fe8a5e) to head (963fa6b).
⚠️ Report is 22 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #2105       +/-   ##
============================================
+ Coverage    32.94%   84.54%   +51.59%     
============================================
  Files           75      158       +83     
  Lines         7716    12945     +5229     
  Branches      1469     3224     +1755     
============================================
+ Hits          2542    10944     +8402     
+ Misses        4913      954     -3959     
- Partials       261     1047      +786

☔ View full report in Codecov by Sentry.
📢 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.

coderabbitai · 2026-01-16T20:05:28Z

Walkthrough

Standardizes endianness handling by introducing explicit LE/BE conversion macros (zendian.h) and replacing conditional BYTE_ORDER/ZSWAP usage across CRC, deflate, and inflate code; removes one local gzip header buffer and an include from a template header.

Changes

Cohort / File(s)	Summary
Endianness macros `zendian.h`	Added `Z_U16_TO_LE/Z_U32_TO_LE/Z_U64_TO_LE`, `Z_U16_FROM_LE/Z_U32_FROM_LE/Z_U64_FROM_LE`, `Z_U16_TO_BE/Z_U32_TO_BE/Z_U64_TO_BE`, `Z_U16_FROM_BE/Z_U32_FROM_BE/Z_U64_FROM_BE` with `BYTE_ORDER` dispatch.
Word / braid macros `crc32_braid_p.h`	Removed `ZSWAPWORD`; added `Z_WORD_FROM_LE(word)` mapping to `Z_U64_FROM_LE` on `ARCH_64BIT` or `Z_U32_FROM_LE` otherwise.
CRC braid (generic) `arch/generic/crc32_braid_c.c`	Replaced `ZSWAPWORD` uses with `Z_WORD_FROM_LE` for CRC init and final combine.
CRC chorba (generic) `arch/generic/crc32_chorba_c.c`	Replaced per-path `BYTE_ORDER`/swap logic with uniform `Z_U64_FROM_LE`/`Z_U32_FROM_LE` for inputs and `Z_U64_TO_LE`/`Z_U32_TO_LE` for final XORs across all paths.
Deflate write helpers `deflate.h`	`put_short`, `put_short_msb`, `put_uint32`, `put_uint32_msb`, `put_uint64` now use unconditional `Z_Ux_TO_LE`/`Z_Ux_TO_BE` conversions instead of `#if BYTE_ORDER` branches.
Inflate helpers / header `inflate_p.h`, `inflate.c`	`inflate_p.h`: added `zendian.h`; `CRC2/CRC4` pass little-endian temporaries to `crc32`; `load_64_bits` uses `Z_U64_FROM_LE`. `inflate.c`: removed local 4-byte gzip header buffer `hbuf`.
Deflate implementations & templates `deflate_fast.c`, `deflate_quick.c`, `insert_string_tpl.h`	Replaced host-endian dependent 4‑byte reads with `Z_U32_FROM_LE(zng_memread_4(...))`; removed `BYTE_ORDER`-conditioned read macros (e.g., `HASH_CALC_READ`).
Header cleanup `inffast_tpl.h`	Removed `#include "zendian.h"` from this template header.

Sequence Diagram(s)

(Skipped — changes are refactorings of endianness handling, not a new multi-component control-flow feature.)

Possibly related PRs

Refactor crc32_fold functions into single crc32_copy #2048: Modifies the same CRC32 implementation files and touches CRC braid/chorba code — likely overlapping endianness/CRC work.
Rewrite chorba dispatch functions #2006: Also updates arch/generic/crc32_chorba_c.c (dispatch/alignment changes) and may conflict or overlap with these endianness changes.

Suggested labels

Compatibility

Suggested reviewers

Dead2
KungFuJesus

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making this check inconclusive as there is no content to evaluate against the changeset.	Add a brief description explaining the motivation and scope of replacing conditional byte-swapping with portable LE conversion macros.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main objective of the pull request: replacing conditional byte-swapping logic with portable endianness conversion macros across the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

inflate.c

zbuild.h

coderabbitai · 2026-01-16T20:08:48Z

Walkthrough

Standardizes endianness handling across the codebase by introducing explicit host-to-little-endian and host-to-big-endian conversion macros in zbuild.h, then updating CRC, deflate, and inflate implementations to use these new macros instead of conditional ZSWAP operations.

Changes

Cohort / File(s)	Summary
Endianness macro foundation `zbuild.h`	Added 12 new conversion macros: ZHTOLE16/32/64, ZLE16/32/64TOH, ZHTOBE16/32/64, ZBE16/32/64TOH; each conditionally defined based on BYTE_ORDER to map to identity or ZSWAP operations as appropriate.
CRC braid implementation `arch/generic/crc32_braid_c.c`, `crc32_braid_p.h`	Updated initialization and finalization of braided CRC accumulators to use ZHTOLEW/ZLEWTOH macros instead of ZSWAPWORD; macro definitions in header now explicitly define these conversions for each endian branch.
CRC Chorba implementation `arch/generic/crc32_chorba_c.c`	Replaced multiple conditional byte-order swap blocks (BYTE_ORDER checks, ZSWAP64/32 usage) with unconditional ZHTOLE/ZLE conversion macros; affects initialization of next1/next1_64 temporaries, per-path byte loading, and final value handling across 64-bit, 32-bit, and small-nondestructive variants.
Deflate output helpers `deflate.h`	Updated put_short, put_short_msb, put_uint32, put_uint32_msb, and put_uint64 inline helpers to unconditionally use ZHTOLE/ZHTOBE conversions instead of conditional BYTE_ORDER-based swapping.
Inflate header and implementation `inflate_p.h`, `inflate.c`	Replaced CRC2/CRC4 buffer writes and load_64_bits endian conditional with explicit ZHTOLE16/32 temporary conversions and ZLE64TOH macro; removed unused local gzip header buffer declaration.

Possibly related PRs

Clean up crc32_braid. #1873: Modifies endian conversion macros in the same CRC braid and Chorba implementation files.
Fix 32bit large chorba #1912: Updates word-size and endian handling in crc32_chorba_c.c with related swap macro changes.
Refactor crc32_fold functions into single crc32_copy #2048: Refactors crc32_braid code paths and adjusts endianness handling in the same CRC implementation files.

Suggested labels

Architecture

Suggested reviewers

Dead2
KungFuJesus

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether a description exists and relates to the changeset.	Add a pull request description explaining the motivation for replacing conditional byte swapping with portable conversions and any testing performed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: replacing conditional byte-swapping logic with portable host/little-endian conversion macros across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mtl1979 · 2026-01-16T20:16:04Z

I don't think the new naming is more readable... At bare minimum it needs some underscores.

nmoinvaz · 2026-01-16T20:22:34Z

~~Yeah I think I will switch to the ZREAD prefix which is more readable.~~ I tried it. Wasn't so.

inflate_p.h

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

inflate_p.h (1)
8-9: Missing #include "zendian.h" causes build failure.

The file uses ZHTOLE16, ZHTOLE32, and ZLE64TOH macros defined in zendian.h, but does not include this header. This causes the pipeline failure: "Call to undeclared function 'ZLE64TOH'".
🐛 Proposed fix
 `#include` <stdlib.h>
+#include "zendian.h"
 `#include` "zmemory.h"

mtl1979 · 2026-01-16T22:05:35Z

~~Yeah I think I will switch to the ZREAD prefix which is more readable.~~ I tried it. Wasn't so.

Abreviating "WORD" to "W" makes the two macro names least readable... I have nothing against using numeric bit widths in the other macros as the numbers acts as natural separator.

zendian.h

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

arch/generic/crc32_chorba_c.c (1)

1-1423: Endianness handling uses portable macros correctly, but the characterization of two distinct strategies is inaccurate.

The refactoring correctly replaces conditional byte-swapping with ZLETOH*/ZHTOLE* macros defined in zendian.h. However, all functions use input-element conversion—they differ only in when it occurs:

118960: Delays ZLETOH64 conversion to the final loop; first two loops work with unconverted z_word_t values

32768 and small variants: Convert each input element with ZLETOH64/ZLETOH32 in their main loops

All functions correctly convert CRC/state values to LE format via ZHTOLE64/ZHTOLE32 before final buffer operations. The macros properly handle both little-endian and big-endian platforms (expanding to no-ops on LE, ZSWAP* on BE).

zendian.h

arch/generic/crc32_chorba_c.c

mtl1979

After merging assignment with optional byteswap, the following XOR operations can be merged to same line. The macro invocation should not need parentheses around to make order of operations correct, but this should be verified, and if needed the macro invocation should be surrounded by parentheses.

mtl1979 · 2026-01-17T00:47:31Z

Most of the failures on CI are big-endian architectures, so possibly line 49 is bad...

nmoinvaz · 2026-01-17T01:24:52Z

Most of the failures on CI are big-endian architectures, so possibly line 49 is bad...

Yeah I think Coderabbit was wrong on this one.

mtl1979 · 2026-01-17T01:33:11Z

Yeah I think Coderabbit was wrong on this one.

It can't really give good suggestions if it doesn't know the naming rules/guidelines for preprocessor macros and the correct macro doesn't already exist...

crc32_braid_p.h

mtl1979 · 2026-01-17T03:13:59Z

CodeCov shows failures most likely due to PR needing rebase, currently there is 2 commits missing from base...

pps83 · 2026-01-18T16:49:07Z

The current naming style follows posix conversion, but another alternative that is probably more readable would be ZREADLE16/32/64.

perhaps, could take what ffmpeg/avutil has for naming

zendian.h

nmoinvaz · 2026-01-21T16:29:29Z

Found a few more instances to replace.

zendian.h

nmoinvaz · 2026-01-22T19:55:22Z

Rebased.

coveralls · 2026-01-22T20:24:00Z

coverage: 94.6% (-0.1%) from 94.7%
when pulling 15bfa0e on nmoinvaz:cleanup/zswap-endian
into 3912768 on zlib-ng:develop.

Dead2

LGTM

nmoinvaz · 2026-01-23T18:21:19Z

Thanks!

nmoinvaz added the cleanup Improving maintainability or removing code. label Jan 16, 2026

nmoinvaz force-pushed the cleanup/zswap-endian branch from 59ba4d1 to 983d218 Compare January 16, 2026 19:54

coderabbitai bot reviewed Jan 16, 2026

View reviewed changes

inflate.c Outdated Show resolved Hide resolved

zbuild.h Outdated Show resolved Hide resolved

nmoinvaz force-pushed the cleanup/zswap-endian branch from 983d218 to 3999d2e Compare January 16, 2026 21:39

github-advanced-security bot found potential problems Jan 16, 2026

View reviewed changes

inflate_p.h Fixed Show fixed Hide fixed

coderabbitai bot reviewed Jan 16, 2026

View reviewed changes

nmoinvaz force-pushed the cleanup/zswap-endian branch 2 times, most recently from 41e9330 to 72dab38 Compare January 16, 2026 21:48

phprus reviewed Jan 16, 2026

View reviewed changes

zendian.h Outdated Show resolved Hide resolved

nmoinvaz force-pushed the cleanup/zswap-endian branch from 72dab38 to 9448a81 Compare January 16, 2026 22:28

coderabbitai bot reviewed Jan 16, 2026

View reviewed changes

zendian.h Outdated Show resolved Hide resolved

mtl1979 reviewed Jan 16, 2026

View reviewed changes