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

dict: embed item structs in hash entries #71

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

bgilbert
Copy link
Collaborator

@bgilbert bgilbert commented Oct 9, 2023

Instead of duplicating an item struct's fields in the hash entry struct that wraps it, add the entire item struct as a field of the hash entry. This eliminates casts and ensures the type system understands what we're doing.

Fixes strict aliasing violations in dcm_init(): we were assigning through a cast and then immediately reading through the original pointer. This happened to cause segfaults on RHEL 9 aarch64, but any compiler would be entitled to optimize away the read.

Also rename some variables to clarify that they're not _hash_entry structs.

Instead of duplicating an item struct's fields in the hash entry struct
that wraps it, add the entire item struct as a field of the hash entry.
This eliminates casts and ensures the type system understands what we're
doing.

Fixes strict aliasing violations in dcm_init(): we were assigning through
a cast and then immediately reading through the original pointer.  This
happened to cause segfaults on RHEL 9 aarch64, but any compiler would be
entitled to optimize away the read.
Clarify that they're not _hash_entry structs.
@jcupitt
Copy link
Collaborator

jcupitt commented Oct 9, 2023

Thanks!

@jcupitt jcupitt merged commit fae3db0 into ImagingDataCommons:main Oct 9, 2023
5 checks passed
@bgilbert bgilbert deleted the aliasing branch October 9, 2023 15:15
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