Skip to content

Fix unsafe_op_in_unsafe_fn warning in AArch64 CRC32 implementation #334

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

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

brian-pane
Copy link

No description provided.

@brian-pane brian-pane force-pushed the crc32 branch 2 times, most recently from 4f11cf8 to 767a798 Compare April 4, 2025 18:42
@brian-pane
Copy link
Author

Did the CI recently upgrade to a newer version of Clippy? There was a lint error on some existing code, but I included a fix for it.

Copy link
Collaborator

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah a new rust release usually means a couple of new clippy lints, so that happened last week

Comment on lines 13 to 14
// SAFETY: `remainder` verifies that `before` contains enough bytes before dereferencing
c = unsafe { remainder(c, before) };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is not what the usafety of remainder is about. The function is unsafe only because it uses a target feature, and at least for our minimum supported rust version, that implies that the function must be unsafe. In this caller we also enable/assume the crc target feature, and so there are no other preconditions to satisfy.

@folkertdev folkertdev merged commit b623176 into trifectatechfoundation:main Apr 7, 2025
22 checks passed
@brian-pane brian-pane deleted the crc32 branch April 10, 2025 17:06
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.

3 participants