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

Avoid popping API context when one wasn't pushed #848

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

jhendersonHDF
Copy link
Collaborator

@jhendersonHDF jhendersonHDF commented Jul 21, 2021

This PR avoids popping an API context when one wasn't pushed, which is usually the case when library initialization fails. If library initialization fails, the API context push will be skipped and then when H5CX_pop is hit inside the FUNC_LEAVE_ macros, an assertion will be triggered because it expects a context to have been pushed.

\
BEGIN_MPE_LOG

/* Use this macro for all "normal" API functions */
#define FUNC_ENTER_API(err) \
{ \
{ \
hbool_t api_ctx_pushed = FALSE; \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

While I'd like to have this variable restricted to the scope of FUNC_ENTER_API_PUSH, GCC gives false positives about possible use of an uninitialized variable unless it's at this broader scope. I suspect a ton of macro refactoring would be needed to straighten GCC's situation out.

@@ -2369,14 +2375,17 @@ H5_DLL herr_t H5CX_pop(hbool_t update_dxpl_props);
H5_API_SET_CANCEL

#define FUNC_LEAVE_API_COMMON(ret_value) \
; \
} /*end scope from end of FUNC_ENTER*/ \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This doesn't belong here, as FUNC_ENTER_API_COMMON doesn't leave the trailing opening brace, it's the individual FUNC_ENTER_ macros that do. Including the brace here can mess up the takedown ordering inside the FUNC_LEAVE_ macros, so the changes here move back to duplicating the closing brace in each FUNC_LEAVE_ macro rather than trying to keep it in a common place.

@lrknox lrknox merged commit 96b6872 into HDFGroup:develop Jul 22, 2021
@jhendersonHDF jhendersonHDF deleted the api_context_push_check branch January 12, 2022 23:19
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Mar 25, 2022
lrknox pushed a commit that referenced this pull request Mar 25, 2022
* Use internal version of H5Eprint2 to avoid possible stack overflow (#661)

* Add support for parallel filters to h5repack (#832)

* Allow parallel filters feature for comm size of 1 (#840)

* Avoid popping API context when one wasn't pushed (#848)

* Fix several warnings (#720)

* Don't allow H5Pset(get)_all_coll_metadata_ops for DXPLs (#1201)

* Fix free list tracking and cleanup cast alignment warnings (#1288)

* Fix free list tracking and cleanup cast alignment warnings

* Add free list tracking code to H5FL 'arr' routines

* Fix usage of several HDfprintf format specifiers after HDfprintf removal (#1324)

* Use appropriate printf format specifiers for haddr_t and hsize_t types directly (#1340)

* Fix H5ACmpio dirty bytes creation debugging (#1357)

* Fix documentation for H5D_space_status_t enum values (#1372)

* Parallel rank0 deadlock fixes (#1183)

* Fix several places where rank 0 can skip past collective MPI operations on failure

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix a few issues noted by LGTM (#1421)

* Fix cache sanity checking code by moving functions to wider scope (#1435)

* Fix metadata cache bug when resizing a pinned/protected entry (v2) (#1463)

* Disable memory alloc sanity checks by default for Autotools debug builds (#1468)

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Apr 13, 2022
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.

4 participants