Remove TRACE macros #4341

derobins · 2024-04-07T19:39:09Z

Removes the TRACE macros from src/H5*.c and updates bin/trace to only handle H5ARG_TRACE macros.

We handle volatile, which is unused, so might as well do restrict

This is kind of crude right now, and the script definitely needs some extra work

* Fix unused param macro in H5I.c call * Remove H5_ATTR_DEBUG_API_USED * Remove trace handling in FUNC_ENTER/LEAVE macros

derobins · 2024-04-07T19:39:56Z

.clang-format

  - FUNC_ENTER_NOAPI
  - FUNC_ENTER_NOAPI_NOERR
  - FUNC_ENTER_NOAPI_NOINIT
  - FUNC_ENTER_NOAPI_NOINIT_NOERR
  - FUNC_ENTER_PACKAGE
  - FUNC_LEAVE_API
+  - FUNC_LEAVE_API_NAMECHECK_ONLY


These were causing clang-format to mess up H5TS.c

derobins · 2024-04-07T19:46:05Z

The handing of H5ARG_TRACE isn't great. I've left it unchanged from the current behavior, but it's going to need some further attention. For example, right now it can't insert an H5ARG_TRACE macro, only update an existing one.

I'll remove H5_trace() in a future PR

derobins · 2024-04-07T19:48:33Z

bin/trace

Probably easier to view this file by itself than as a diff. Also probably want to turn whitespace off since I made the indents uniform.

jhendersonHDF · 2024-04-07T21:13:37Z

src/H5I.c

@@ -90,14 +90,13 @@ static int H5I__iterate_pub_cb(void *obj, hid_t id, void *udata);
 *-------------------------------------------------------------------------
 */
 H5I_type_t
-H5Iregister_type(size_t H5_ATTR_DEBUG_API_USED hash_size, unsigned reserved, H5I_free_t free_func)
+H5Iregister_type(size_t H5_ATTR_UNUSED hash_size, unsigned reserved, H5I_free_t free_func)


Interesting. Very weird to have an unused parameter in an API function.

It's because the original implementation of ID storage was a hash table that took a size parameter. It's an internal implementation detail that never should have been exposed. We should remove it and version the API call for 2.0.

jhendersonHDF · 2024-04-07T21:21:05Z

Good cleanup. Two things though:

This still seems like something that would be good to announce prior to removal, in line with https://github.com/HDFGroup/hdf5/wiki/Change-Policy, as I've seen recent examples of people still using this feature.
There are probably additional things to update alongside this change, such as the entire section on API tracing in https://github.com/HDFGroup/hdf5/blob/develop/doxygen/examples/DebuggingHDF5Applications.html.

derobins · 2024-04-07T21:28:29Z

Good cleanup. Two things though:

This still seems like something that would be good to announce prior to removal, in line with https://github.com/HDFGroup/hdf5/wiki/Change-Policy, as I've seen recent examples of people still using this feature.

There are probably additional things to update alongside this change, such as the entire section on API tracing in https://github.com/HDFGroup/hdf5/blob/develop/doxygen/examples/DebuggingHDF5Applications.html.

Yes. That guide should be reworked, but I was going to save that for v 2.0 of these changes. I'd like to have the bin/trace script be smarter about pasting the macros in, for example. I only want to update the guide once.

We can announce the change.

qkoziol · 2024-04-07T21:47:01Z

Good cleanup. Two things though:

This still seems like something that would be good to announce prior to removal, in line with https://github.com/HDFGroup/hdf5/wiki/Change-Policy, as I've seen recent examples of people still using this feature.

There are probably additional things to update alongside this change, such as the entire section on API tracing in https://github.com/HDFGroup/hdf5/blob/develop/doxygen/examples/DebuggingHDF5Applications.html.

Yes. That guide should be reworked, but I was going to save that for v 2.0 of these changes. I'd like to have the bin/trace script be smarter about pasting the macros in, for example. I only want to update the guide once.

We can announce the change.

Agree that we should announce this change.

qkoziol

''All code shall yield to the mighty deleter-of-lines!'' ;-)

* Fixed dead links in docs (#4334) * Revert "Fixed dead links in docs (#4334)" (#4335) This reverts commit 408fc66. * Fix grammar and content in HDF5Examples (#4333) * Remove HD macros for C time functions (#4337) * HDasctime * HDclock * HDctime * HDdifftime * HDgmtime * HDlocaltime * HDmktime * HDtime * HDtzset HDgettimeofday will be done later * Remove HD prefix from toupper/tolower (#4340) * Add VDS and SWMR to documentation (#4336) * Clean up random number generator code (#4338) * Clean up random number generator code Depending on the platform, we use a mix of random, rand, and rand_r to generate pseudo-random numbers, along with a messy set of ifdefs in H5private.h. We are not a cryptographic library, only use random numbers in our test code, and have no need for anything more than the C standard's (s)rand(). There's no point dithering about using rand() vs random() when we're also doing bad things like using mod to restrict the range, which introduces bias. Also removes CMake/configure checks for rand_r and random * Remove random/rand_r checks from build system * Fix missed HDrandom after GitHub merge * Remove the getting started guide (#4339) This was moved to the wiki * Remove noise comment from H5DO.c (#4342) * Add note about H5Iregister_type hash_table param (#4345) The hash_table parameter is unused in HDF5 1.8.13 and later, when we moved from using hash tables to skip lists for storing IDs. The current hash table implementation does not use this parameter. * Remove `#undef ERR` from H5private.h (#4346) This is an old work-around that is no longer necessary. The only place where we defined ERR is in h5import, and that code has been updated (in this PR) to rename ERR to INVALID_TOKEN. * Add NAMECHECK_ONLY to macro list in .clang-format (#4343) This caused H5TS.c functions to be mis-formatted * Remove HD from last non-win32 C89 functions (#4348) Removes the HD prefix from the last C89 functions with no special Windows equilvalent: * rename * rewind * setbuf * signal * tmpfile * Fix dead links cont. (#4349) Added img/images_to_copy.dox as a temporary solution because doxygen didn't copy the images used in the examples/*.html files - will investigate more. This was necessary for the links to intro_SWMR.html and intro_VDS.html. * Fix a typo (#4352) * Fix use of possibly uninitialized memory in h5repack (#4354) * Match content listing with content. (#4357) * Remove obsolete c++ doxygen config files (#4358) * Fix typo VERSIO (#4363) * Removed duplicate entries in compile line for h5* compiler wrappers (#4360) * h5fc remove duplicate args * h5 wrappers remove duplicate args * Added CVE info to RELEASE.txt (#4367) * Change doxygen version from 1.10.0 to 1.9.7 (#4371) * Fix all doxygen versions (#4377) * Change date to data in INSTALL_parallel (#4384) * Fix a broken link for zlib url (#4383) * Fix broken links and references in INSTALL_Auto.txt (#4382) * Remove signature in comment (#4366) * switched from \returns to commonly used \return (#4389) * Add CXX flags (#4359) * Remove TRACE macros (#4341) * Set available CMake build types for non-multi-config generators (#4330) * Correct the high-level library link (#4398) * Correct the high-level library link The link currently used points to 1.13 release. Changed to use the md file instead. * Update Doxyfile for md files * Update to include md files * Add additional terms to the glossary (#4401) Fixes GH #4390 * added missing doxygen \since (#4410) * Update Intel oneAPI version to 2024.1 (#4412) * Update documentation (#4403) - Added File Image Operations to Technical Notes * Use a correct example location * Use aliases for URLs (#4409) * Fix CXX sanitizer by moving marco include to after CXX code. (#4414) * Update and use more URL alias (#4413) * Update documentation (#4415) Added the replacement for https://docs.hdfgroup.org/archive/support/HDF5/doc/TechNotes/Version.html Corrected some minor typos and another deadlink * Remove obsolete file (#4416) * Update oneAPI version to 2024.1 for Daily Build (#4418) * Add training videos page (#4422) * Updates for release_docs/NEWSLETTER.txt. (#4423) * Add release process markdown file (#4312) * Lowercase in File Format Specification (#4424) * Fixing string in RELEASE_PROCESS.md that is causing a doxygen error - (#4430) Unsupported xml/html tag <n> found. * Allow clang-format to format H5Cpkg.h (#4429) Co-authored-by: Larry Knox <lrknox@hdfgroup.org> * Fix broken links in md files needing full URLs. * Add URL for Java examples that was accidentally dropped. Change paths from develop to 1.14 directories for some doxygen generated files. * Correct typo. * Fix 2 more dead URLs. * Revert "Clean up random number generator code (#4338)" This reverts commit 58e4d08.

derobins added 21 commits April 7, 2024 10:51

bin/trace indents are now 4 spaces

366c22c

Handle restrict type qualifier

92ab47b

We handle volatile, which is unused, so might as well do restrict

Remove TRACE macro addition

a054008

This is kind of crude right now, and the script definitely needs some extra work

Removed NO TRACE comments

ddc9c73

Removed H5TRACE lines from source

d03b9b4

Removed noise comment in H5DO.c

dd61ac1

Remove H5TRACE macros from H5private.h

39120e0

Update autogen.sh comment

96d18a2

Update bin/README.md

dc4f4b5

Remove API counts from output

d6df14b

Removed $trace

43f553f

Remove $rettype

d7bb8cb

Only count changed H5ARG_TRACE macros

64cbfae

Only exclude "external" source files

d44a302

Capital I looks nicer

b18ff8b

Removed length limit since that no longer matters

169a4b1

Grammaring

fdb61bd

Add a helpful comment to the script

ce72608

Minor commenting

82f57f5

H5private.h trace macro cleanup

f9ea713

* Fix unused param macro in H5I.c call * Remove H5_ATTR_DEBUG_API_USED * Remove trace handling in FUNC_ENTER/LEAVE macros

Add NAMECHECK_ONLY macros to .clang-format

9ab3e17

derobins requested review from lrknox, byrnHDF, fortnern, jhendersonHDF and qkoziol as code owners April 7, 2024 19:39

derobins requested review from vchoi-hdfgroup, bmribler, glennsong09, mattjala and brtnfld as code owners April 7, 2024 19:39

derobins commented Apr 7, 2024

View reviewed changes

derobins changed the title ~~Remove h5 trace~~ Remove TRACE macros Apr 7, 2024

derobins commented Apr 7, 2024

View reviewed changes

jhendersonHDF reviewed Apr 7, 2024

View reviewed changes

qkoziol approved these changes Apr 7, 2024

View reviewed changes

qkoziol reviewed Apr 7, 2024

View reviewed changes

jhendersonHDF approved these changes Apr 11, 2024

View reviewed changes

derobins merged commit 125b243 into HDFGroup:develop Apr 12, 2024
58 checks passed

qkoziol pushed a commit to qkoziol/hdf5 that referenced this pull request Apr 13, 2024

Remove TRACE macros (HDFGroup#4341)

89ac260

lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Apr 22, 2024

Remove TRACE macros (HDFGroup#4341)

7dab259

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove TRACE macros #4341

Remove TRACE macros #4341

derobins commented Apr 7, 2024 •

edited

Loading

derobins Apr 7, 2024

derobins commented Apr 7, 2024

derobins Apr 7, 2024

jhendersonHDF Apr 7, 2024

derobins Apr 7, 2024

jhendersonHDF commented Apr 7, 2024

derobins commented Apr 7, 2024

qkoziol commented Apr 7, 2024

qkoziol left a comment

Remove TRACE macros #4341

Remove TRACE macros #4341

Conversation

derobins commented Apr 7, 2024 • edited Loading

derobins Apr 7, 2024

Choose a reason for hiding this comment

derobins commented Apr 7, 2024

derobins Apr 7, 2024

Choose a reason for hiding this comment

jhendersonHDF Apr 7, 2024

Choose a reason for hiding this comment

derobins Apr 7, 2024

Choose a reason for hiding this comment

jhendersonHDF commented Apr 7, 2024

derobins commented Apr 7, 2024

qkoziol commented Apr 7, 2024

qkoziol left a comment

Choose a reason for hiding this comment

derobins commented Apr 7, 2024 •

edited

Loading