Fix usage of several HDfprintf format specifiers after HDfprintf removal #1324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhendersonHDF
requested review from
derobins,
fortnern,
jrmainzer,
qkoziol,
soumagne and
vchoi-hdfgroup
as code owners
jhendersonHDF
force-pushed
the
HDfprintf_old_format_fixes
branch
from
e6fe300
to
4f4df10
Compare
byrnHDF
approved these changes
Jan 4, 2022
lrknox
approved these changes
Jan 4, 2022
gnuoyd
suggested changes
Jan 4, 2022
| HDfprintf(stderr, "%s: Opening free space manager, fs_addr = %a, nclasses = %Zu\n", __func__, fs_addr, | ||
| nclasses); | ||
| HDfprintf(stderr, "%s: Opening free space manager, fs_addr = " H5_PRINTF_HADDR_FMT ", nclasses = %Zu\n", | ||
| __func__, fs_addr, nclasses); |
There was a problem hiding this comment.
I intended for "%" PRIuHADDR to replace "%a" to be consistent with the C convention—PRIu64, etc.
Let's deprecate H5_PRINTF_HADDR_FMT.
| HDfprintf(stderr, "%s: fspace->sect_addr = " H5_PRINTF_HADDR_FMT "\n", __func__, fspace->sect_addr); | ||
| HDfprintf(stderr, "%s: fspace->sect_size = " H5_PRINTF_HSIZE_FMT "\n", __func__, fspace->sect_size); | ||
| HDfprintf(stderr, "%s: fspace->alloc_sect_size = " H5_PRINTF_HSIZE_FMT "\n", __func__, | ||
| fspace->alloc_sect_size); |
There was a problem hiding this comment.
Likewise, "%" PRIuHSIZE here.
| #ifdef H5FA_DEBUGGING | ||
| #endif /* H5FA_DEBUGGING */ | ||
| #ifdef H5FA_DEBUG | ||
| #endif /* H5FA_DEBUG */ |
|
Sorry, Dave, I merged this PR just before your request for changes came through. Do you want me to revert the merge, or to just create a new PR deprecating H5_PRINTF_HADDR_FM and making the "%" PRIuHADDR, "%" PRIuHSIZE, and other changes?
Library developers: there are a number of PRs to remove clang warnings that need your review. Most of them are just one to a few lines. I think the warning in #1129 was fixed by #ll69 and if I can verify that I will close #1129. PRs number #1299 and later need review. Some of the older ones need discussion and resolution.
PR #1302: tests of jhendersonHDF:parallel_filters<https://github.com/jhendersonHDF/hdf5/tree/parallel_filters> with OpenMPI have a few parallel tests that fail. I’ll add a list to the PR shortly.
Larry
From: David Young ***@***.***>
Sent: Tuesday, January 04, 2022 11:05 AM
To: HDFGroup/hdf5 ***@***.***>
Cc: Larry Knox ***@***.***>; State change ***@***.***>
Subject: Re: [HDFGroup/hdf5] Fix usage of several HDfprintf format specifiers after HDfprintf removal (PR #1324)
@gnuoyd requested changes on this pull request.
________________________________
In src/H5FS.c<#1324 (comment)>:
@@ -180,8 +181,8 @@ H5FS_open(H5F_t *f, haddr_t fs_addr, uint16_t nclasses, const H5FS_section_class
FUNC_ENTER_NOAPI(NULL)
#ifdef H5FS_DEBUG
- HDfprintf(stderr, "%s: Opening free space manager, fs_addr = %a, nclasses = %Zu\n", __func__, fs_addr,
- nclasses);
+ HDfprintf(stderr, "%s: Opening free space manager, fs_addr = " H5_PRINTF_HADDR_FMT ", nclasses = %Zu\n",
+ __func__, fs_addr, nclasses);
I intended for "%" PRIuHADDR to replace "%a" to be consistent with the C convention—PRIu64, etc.
Let's deprecate H5_PRINTF_HADDR_FMT.
________________________________
In src/H5FS.c<#1324 (comment)>:
@@ -201,10 +202,11 @@ H5FS_open(H5F_t *f, haddr_t fs_addr, uint16_t nclasses, const H5FS_section_class
(fspace = (H5FS_t *)H5AC_protect(f, H5AC_FSPACE_HDR, fs_addr, &cache_udata, H5AC__READ_ONLY_FLAG)))
HGOTO_ERROR(H5E_FSPACE, H5E_CANTPROTECT, NULL, "unable to load free space header")
#ifdef H5FS_DEBUG
- HDfprintf(stderr, "%s: fspace->sect_addr = %a\n", __func__, fspace->sect_addr);
- HDfprintf(stderr, "%s: fspace->sect_size = %Hu\n", __func__, fspace->sect_size);
- HDfprintf(stderr, "%s: fspace->alloc_sect_size = %Hu\n", __func__, fspace->alloc_sect_size);
- HDfprintf(stderr, "%s: fspace->sinfo = %p\n", __func__, fspace->sinfo);
+ HDfprintf(stderr, "%s: fspace->sect_addr = " H5_PRINTF_HADDR_FMT "\n", __func__, fspace->sect_addr);
+ HDfprintf(stderr, "%s: fspace->sect_size = " H5_PRINTF_HSIZE_FMT "\n", __func__, fspace->sect_size);
+ HDfprintf(stderr, "%s: fspace->alloc_sect_size = " H5_PRINTF_HSIZE_FMT "\n", __func__,
+ fspace->alloc_sect_size);
Likewise, "%" PRIuHSIZE here.
________________________________
In src/H5FAprivate.h<#1324 (comment)>:
@@ -134,7 +134,7 @@ H5_DLL herr_t H5FA_patch_file(H5FA_t *fa, H5F_t *f);
H5_DLL herr_t H5FA_get_stats(const H5FA_t *ea, H5FA_stat_t *stats);
/* Debugging routines */
-#ifdef H5FA_DEBUGGING
-#endif /* H5FA_DEBUGGING */
+#ifdef H5FA_DEBUG
+#endif /* H5FA_DEBUG */
Just delete?
—
Reply to this email directly, view it on GitHub<#1324 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADGMWCS6LD7CI4P2BEQUDSLUUMSETANCNFSM5KTDJ5AA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you modified the open/close state.Message ID: ***@***.******@***.***>>
|
I'll address these issues in a separate PR later this week. Since H5_PRINTF_HADDR_FMT is already in H5public.h, best not to touch it. I'll remove the new H5_PRINTF_HSIZE_FMT though. |
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
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.
This PR fixes usage of old HDfprintf format specifiers like %a, %t and %Hu after the internal HDfprintf implementation was removed.