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

Export functions isAllocated and isAllocatedMutable from MemModel. #420

Closed
wants to merge 1 commit into from

Conversation

brianhuffman
Copy link
Contributor

No description provided.

Copy link
Contributor

@langston-barrett langston-barrett left a comment

Choose a reason for hiding this comment

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

Thanks for adding the comment!

@brianhuffman
Copy link
Contributor Author

Oops, I forgot to include a couple of lines in the commit before; should be fixed now.

@brianhuffman
Copy link
Contributor Author

Hmm. I've changed my mind; I now think that isAllocated and isAllocatedMutable (and also isAligned) are actually the wrong functions to export. All of these functions have something to do with alignment checks, but they check different parts of the pointer (isAllocated checks only the base part and isAligned checks only the offset part).

I think it would be less confusing, and less likely for people to use the API incorrectly, if we keep all three of these functions internal, and instead export one or two new ones:

  • isReadablePointer would combine isAllocated and isAligned to check whether a pointer is valid, allocated, and properly aligned for reading
  • isWritablePointer would combine isAllocatedMutable and isAligned to check whether a pointer is valid, aligned, and points to a mutable allocated region

I'll close this PR now, and open a separate one with these new functions in it.

@travitch travitch deleted the bh-exports branch August 5, 2022 17:09
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.

None yet

3 participants