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.

Sign up for GitHub

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

Jump to bottom

Suppress cast-qual warning in H5TB Fortran wrapper #3728

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Suppress cast-qual warning in H5TB Fortran wrapper #3728

merged 1 commit into from
Oct 20, 2023

Conversation

derobins
Copy link
Member

This interface is fundamentally broken, const-wise.

@derobins
Suppress cast-qual warning in H5TB Fortran wrapper
f8d249a 
This interface is fundamentally broken, const-wise.
@derobins derobins added Merge - To 1.14 Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Component - Fortran Fortran wrappers Type - Improvement Improvements that don't add a new feature or functionality labels Oct 19, 2023
@derobins
Copy link
Member Author

https://stackoverflow.com/questions/14562845/why-does-passing-char-as-const-char-generate-a-warning

jhendersonHDF
jhendersonHDF approved these changes Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@jhendersonHDF jhendersonHDF left a comment

Choose a reason for hiding this comment

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

Fine for now, though we should eventually go fix all these. The interface is correct; the code isn't technically

@brtnfld
Copy link
Contributor

brtnfld commented Oct 19, 2023

It's probably best to call the C API from Fortran and eliminate the broken code.

@jhendersonHDF
Copy link
Collaborator

It's probably best to call the C API from Fortran and eliminate the broken code.

The code can also be fixed easily, the const interface just makes it a little more awkward to interface with and be const-correct

@brtnfld
Copy link
Contributor

brtnfld commented Oct 19, 2023 •
edited
Loading

Are we talking about the C API sig. itself or the use of it in the C wrapper?

@jhendersonHDF
Copy link
Collaborator

Are we talking about the C API sig. itself or the use of it in the C wrapper?

The code that uses it in the wrapper. The C API signature is correct as far as const goes. We're passing in an array of non-modifiable pointers. In fact, const char * const *field_names would be an even better signature since the function doesn't need to modify the entries in field_names either, but that'd be even more awkward to deal with.

@derobins
Copy link
Member Author

Yeah, I'm not super happy with this, but we have a couple of places where we use const foo** and it causes problems and this is how we paper over it. We should definitely tweak the API's consts in a future major release.

@derobins derobins merged commit b916ce2 into HDFGroup:develop Oct 20, 2023
40 checks passed
jhendersonHDF pushed a commit to jhendersonHDF/hdf5 that referenced this pull request Oct 20, 2023
@derobins @jhendersonHDF
Suppress cast-qual warning in H5TB Fortran wrapper (HDFGroup#3728)
58839c9 
This interface is fundamentally broken, const-wise.
derobins added a commit that referenced this pull request Oct 21, 2023
@jhendersonHDF @hyoklee
@byrnHDF @brtnfld @derobins @fortnern @glennsong09 @bmribler
Sync with develop (#3737)
b16808d 
* Correct ld in format strings in cmpd_dset.c (#3697)

Removes clang warnings

* Clean up comments. (#3695)

* Add NVidia compiler support and CI (#3686)

* Work around Theta system issue failure in links test (#3710)

When the Subfiling VFD is enabled, the links test may
try to initialize the Subfiling VFD and call MPI_Init_thread.
On Theta, this appears to have an issue that will cause
the links test to fail. Reworked the test to check for
the same conditions in a more roundabout way that doesn't
involved initializing the Subfiling VFD

* Fix issue with unmatched messages in ph5diff (#3719)

* provide an alternative to mapfile for older bash (#3717)

* Attempt to quiet some warnings with cray compilers. (#3724)

* Fix CMake VOL passthrough tests by copying files to correct directory (#3721)

* Develop intel split (#3722)

* Split intel compiler flags into sub-folders
* Update Intel options for warnings
* Mostly CMake, Autotools needs additional work

* Fixes and adjustments to t_filters_parallel (#3714)

Broadcast number of datasets to create in multi-dataset I/O cases
so that interference with random number generation doesn't cause
mismatches between ranks

Set fill time for datasets to "never" by default and adjust on a
per-test basis to avoid writing fill values to chunks when it's
unnecessary

Reduce number of loops run in some tests when performing multi-dataset
I/O

Fix an issue in the "fill time never" test where data verification
could fill if file space reuse causes application buffers to be
filled with chosen fill value when reading from datasets with
uninitialized storage

Skip multi-chunk I/O test configurations for multi-dataset I/O
configurations when the TestExpress level is > 1 since those
tests can be more stressful on the file system

Disable use of persistent file free space management for now
since it occasionally runs into an infinite loop in the library's
free space management code

* Suppress cast-qual warning in H5TB Fortran wrapper (#3728)

This interface is fundamentally broken, const-wise.

* Add new API function H5Pget_actual_select_io_mode() (#2974)

This function allows the user to determine if the library performed selection I/O, vector I/O, or scalar (legacy) I/O during the last HDF5 operation performed with the provided DXPL. Expanded existing tests to check this functionality.

* Test scripts now execute in-source with creation of tmp dir (#3723)

Fixes a few issues created in #3580:

* Fixes a problem where committed tools test files were deleted when cleaning after an in-source build
* Fixes issues with test file paths in Autotools tools test scripts

* Add -h and --help as flags in h5cc & h5fc (#3729)

Adds these common help flags in addition to -help

* Update the library version matrix for H5Pset_libver_bounds() (#3702)

* Fixed #3524

Added 1.12, 1.14, and 1.16 to the table for libver bounds in the H5Pset_libver_bounds docs.

* Remove references to LIBVER_V116

---------

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Co-authored-by: Allen Byrne <50328838+byrnHDF@users.noreply.github.com>
Co-authored-by: Scot Breitenfeld <brtnfld@hdfgroup.org>
Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Co-authored-by: Neil Fortner <fortnern@gmail.com>
Co-authored-by: Glenn Song <43005495+glennsong09@users.noreply.github.com>
Co-authored-by: bmribler <39579120+bmribler@users.noreply.github.com>
@derobins derobins deleted the suppress_broken_fortran_H5TB_warning branch March 27, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byrnHDF byrnHDF byrnHDF approved these changes

@jhendersonHDF jhendersonHDF jhendersonHDF approved these changes

@lrknox lrknox Awaiting requested review from lrknox lrknox is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@mattjala mattjala Awaiting requested review from mattjala mattjala is a code owner

@brtnfld brtnfld Awaiting requested review from brtnfld brtnfld is a code owner

Assignees
No one assigned
Labels
Component - Fortran Fortran wrappers Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@derobins @brtnfld @jhendersonHDF @byrnHDF