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

Mark various internal functions as unsafe #254

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

rytheo
Copy link
Contributor

@rytheo rytheo commented Aug 10, 2023

Found various functions that should be unsafe since they take raw pointer arguments and must assume they point to valid data.

Made string_from_fixed_bytes panic on invalid UTF-8 instead of marking it unsafe, but marking it unsafe is a valid alternative if the unwrap call significantly reduces performance.

Copy link
Collaborator

@mulimoen mulimoen left a comment

Choose a reason for hiding this comment

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

Looks good, this will allow storing C functions directly in the type without wrapping it up in a "safe" function.

Marking the functions in hdf5/src/util.rs as unsafe is a breaking change. Could this be added to the changelog?

@rytheo
Copy link
Contributor Author

rytheo commented Aug 13, 2023

@mulimoen changing hdf5/src/util.rs shouldn't be a breaking change; looking at hdf5/src/lib.rs, it seems like none of the functions in util are accessible outside the crate. Am I missing something?

@mulimoen
Copy link
Collaborator

@rytheo You are right, after closer examination it is not exported publicly. Note to self: Should mark these functions/modules as pub(crate)

@aldanor aldanor merged commit 8046bae into aldanor:master Jan 30, 2024
40 of 41 checks passed
@rytheo rytheo deleted the mark-unsafe-functions branch January 31, 2024 01:32
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