Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add H5Tdecode2, rename and deprecate H5Tdecode #5213
Add H5Tdecode2, rename and deprecate H5Tdecode #5213
Changes from 14 commits
b33ab75
55c1a68
6da7a5b
d941398
6981bc0
30730d4
b1c6e2d
a53b1ad
239c311
9b2e54a
fd8e1c2
cc0bbc4
b6bf81a
efc00ea
96e5e14
f1a3c47
a6b0c79
a4a1ad0
9e87628
10c1cf4
189d61d
a026ad5
c4644a6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Thank you for addressing the issue, but I'm sorry, I made a mistake in the original comment. If encoded_buf is not NULL then buf_size will already be the size of the encoded buffer. You can safely use buf_size in H5Tdecode2 as you originally did. I'm sorry 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.
Without having to call H5Tencode to get the size, that is. Checking buf_size is a good idea though. However, if you want to put a fix in DataType::close() to set encoded_buf to NULL then checking for NULL encoded_buf alone will be good enough. Question me if you see anything I said questionable. :D
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.
At first I was going to mention that there should be the corresponding JNI changes, but it seems there is no
_H5Tdecode
implementation currently. @byrnHDF We should either implement that or remove this function from being available.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.
Definitely looks like an oversight! I wonder if it got dropped along the way?
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.
Most likely the issue is the byte buffer and translating C memory to java memory
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 find H5Tdecode in the code tag hdf5-1_8_0, but not in tag hdf5-1_6_10, so I think this line should end with ; v18, v200.
I believe that change would also not add lines 151 - 154 in H5version.h.
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.
That's strange. I can't find it in 1.6 either, but the reference manual lists it as
since 1.2
. I'll update both of these to reflect a 1.8 addition.