-
Notifications
You must be signed in to change notification settings - Fork 8
Fix dataset creation failure on Ubuntu 24 #131
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
Conversation
29c720b to
d130105
Compare
862bc1c to
cd9e087
Compare
| strcpy(host_header, host_string); | ||
|
|
||
| curl_headers_local = curl_slist_append( | ||
| curl_headers_local, strncat(host_header, filename, host_header_len - strlen(host_string) - 1)); |
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.
Why were these converted back to strcat calls?
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.
The lengths provided to strncat were pulled from the length of the target strings anyway, so having these be strncat didn't really accomplish anything.
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.
Hmm, in this case yes they weren't doing much, though they're more future-proof than a regular strcat in case someone adds more things to the header in the future and forgets to allocate the space correctly. Really, I think most of these should just be switched to a single snprintf anyway to avoid most of the problems of having to manage the buffer size.
.github/workflows/main.yml
Outdated
| -DHDF5_BUILD_HL_LIB=ON \ | ||
| -DBUILD_SHARED_LIBS=ON -DHDF5_ENABLE_SZIP_SUPPORT=OFF \ | ||
| -DHDF5_TEST_API=ON \ | ||
| -DHDF5_ENABLE_Z_LIB_SUPPORT=OFF \ |
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.
Just a note, depending on which branch you're building this option could either be Z_LIB or ZLIB now. Though it shouldn't be a problem for develop as long as the default stays OFF
A pointer value in
RV_convert_dataspace_shape_to_JSONwas cast to size_t in a way that didn't make sense, resulting in snprintf being provided an essentially random maximum length. For some reason, this only caused problems on Ubuntu 24.A recent update to the github runners changed ubuntu-latest from 22.04 to 24.04.
Merge autotools/cmake CI jobs into one using matrix to reduce duplication
Remove vol-tests submodule
As of hdf5#5170, the vol-tests don't successfully build anymore. These haven't been used for testing since the API tests were moved back into the library, so it makes sense to remove them.
hsize_t