Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix char signedness issue in _crc32c_sw_slicing_by_8() #44

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Jun 30, 2024

Fix _crc32c_sw_slicing_by_8() to use unsigned char for p_buf, to fix incorrect results on platforms with signed char such as SPARC. The code has been casting unsigned char * to char * for no apparent reason, and this broke the bitshifts in the big endian blocks.

Particularly,

crc ^= *(p_buf++) << 16

would be XOR-ed against 0xffee0000 rather than 0x00ee0000.

Fixes #43

Fix `_crc32c_sw_slicing_by_8()` to use `unsigned char` for `p_buf`,
to fix incorrect results on platforms with signed `char` such as SPARC.
The code has been casting `unsigned char *` to `char *` for no apparent
reason, and this broke the bitshifts in the big endian blocks.

Particularly,

    crc ^= *(p_buf++) << 16

would be XOR-ed against `0xffee0000` rather than `0x00ee0000`.

Fixes ICRAR#43
@rtobar
Copy link
Contributor

rtobar commented Jun 30, 2024

Thanks @mgorny, not only for the PR, but also for reporting the issue and doing the root-cause analysis. Changes look fine to me, I agree the cast was completely unnecessary. I'll tag a release shortly.

@rtobar rtobar merged commit d184417 into ICRAR:master Jun 30, 2024
7 checks passed
@rtobar
Copy link
Contributor

rtobar commented Jun 30, 2024

@mgorny 2.4.1 has been tagged, a new release will hopefully appear automatically in PyPI in an hour or less; otherwise tomorrow I'll have a look.

@mgorny mgorny deleted the sparc-fix branch June 30, 2024 16:28
@mgorny
Copy link
Contributor Author

mgorny commented Jun 30, 2024

Thanks!

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.

test_msvc_examples failure on SPARC
2 participants