-
Notifications
You must be signed in to change notification settings - Fork 141
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
Provide high level syntax for dealing with HDF5 files in memory #1077
Conversation
mkitti
commented
Jun 3, 2023
- Fix get_driver for Core
- Enhance creating a file from memory using the Core driver
src/api/helpers.jl
Outdated
buf_ptr_ref = Ref{Ptr{Nothing}}() | ||
buf_len_ref = Ref{Csize_t}(0) | ||
h5p_get_file_image(fapl_id, buf_ptr_ref, buf_len_ref) | ||
unsafe_wrap(Array{UInt8}, Ptr{UInt8}(buf_ptr_ref[]), buf_len_ref[]; own=true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm always a little bit worried about own=true
: I know the docs say to call free
, but I thought that this wasn't correct on Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be safer to do the double call: first call with buf_ptr_ref
as C_NULL
to get the size of the buffer, then allocate it ourselves, and call it again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, i was wrong. You would need to define H5Pset_file_image_callbacks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's strange is that HDF5 explicitly provides H5_FREE_MEMORY
for this reason elsewhere (see #677).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably should use that. I'll check the HDF5 source later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm resolving this for now by only allowing the one-arg version with own=true
if h5p_get_file_image_callbacks
returns a null pointer for image_free
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should still use HDF5.API.h5_free_memory
.