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

Adds method to check of pointer size if bounded #829

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

ankane
Copy link
Contributor

@ankane ankane commented Sep 27, 2020

Hi, thanks for this great library!

This PR adds a method to tell if the pointer size is bounded.

ptr = FFI::Pointer.new(0)
ptr.bounded?              # false
ptr.slice(0, 10).bounded? # true

This can be useful for libraries that accept FFI::Pointer arguments.

def load_image(ptr, ...)
  raise "Cannot load image from pointer with unbounded size" unless ptr.bounded?
  # do something with size ...
end

@eregon
Copy link
Collaborator

eregon commented Sep 28, 2020

This sounds like a good idea to me.

Right now FFI::Pointer#size returns LONG_MAX for unbounded pointers.
But maybe it should return nil instead to not leak that implementation detail?
That could break some things that rely on #size always being an Integer though.

@ankane
Copy link
Contributor Author

ankane commented Sep 28, 2020

Thanks @eregon. I think nil or 0 would be more intuitive. Currently, code that works well with a FFI::MemoryPointer could end up reading too much data if passed an unbounded FFI::Pointer (for C functions that take a pointer and bytesize).

Edit: Somewhat related, it would also be nice to allow a size to be specified on initialization.

@larskanis
Copy link
Member

Returning nil would be more intuitive, but I worry about compatibility. I don't like the name bounded? - maybe because I'm not a native English speaker. How about size_limit? or has_size? .

@ankane
Copy link
Contributor Author

ankane commented Nov 24, 2020

Hey @larskanis, thanks for the response! I thought bounded? might be good since cannot duplicate unbounded memory area is a current error message. Just pushed size_limit? and fixed the merge conflict.

@larskanis larskanis merged commit 7865f3f into ffi:master Nov 25, 2020
@ankane
Copy link
Contributor Author

ankane commented Nov 25, 2020

Thanks @larskanis!

This was referenced Mar 16, 2021
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