Remove memory sanity checks #2164
Conversation
Removed: * The H5get_alloc_stats() API call * The --enable-memory-alloc-sanity-check Autotools configure option * The HDF5_MEMORY_ALLOC_SANITY_CHECK CMake option
derobins
requested review from
lrknox,
byrnHDF,
fortnern,
jhendersonHDF,
ChristopherHogan,
qkoziol,
vchoi-hdfgroup,
bmribler and
raylu-hdf
as code owners
| @@ -55,13 +55,29 @@ New Features | |||
|
|
|||
| (ADB - 2022/08/29, HDFFV-11329) | |||
|
|
|||
| - | |||
| - Removed the --enable-memory-alloc-sanity-check Autotools configure option | |||
There was a problem hiding this comment.
newer notes at top of section, right?
| if (size != 0) | ||
| ret_value = H5MM_realloc(mem, size); | ||
| else if (mem) | ||
| ret_value = H5MM_xfree(mem); |
There was a problem hiding this comment.
I read here https://www.tutorialspoint.com/c_standard_library/c_function_realloc.htm, it said
size − This is the new size for the memory block, in bytes. If it is 0 and ptr points to an existing block of memory, the memory block pointed by ptr is deallocated and a NULL pointer is returned.
Do we need to use H5MM_xfree instead?
| * Purpose: Just like free(3) except null pointers are allowed as | ||
| * arguments, and the return value (always NULL) can be | ||
| * assigned to the pointer whose memory was just freed: | ||
| * Purpose: Just like free(3) except the return value (always NULL) can |
There was a problem hiding this comment.
Is H5MM_xfree still needed? Do any supported systems fail if we remove it (could test by #define H5MM_xfree H5MM_free)?
There was a problem hiding this comment.
If it gets mapped to just plain free then I think we still have several places in the library that would do mem = free which won't compile, so we'd need to update those. But I don't think it's really needed.
There was a problem hiding this comment.
We should be able to convert H5MM_xfree to a macro:
#define H5MM_xfree(A) (H5MM_free(A), NULL)
|
Test that failed was one of the dt_arith tests in the MacOS Clang CMake REL test configuration. There were also 75+ "Failed ..." messages in the same test configuration, mostly in h5repack (and h5dump) tests, but all of those tests passed. |
This is the same test that fails in #1929. @derobins and I need to figure out what's going on there because it looks like we have a memory issue that's been hidden on MacOS for a long time. |
|
Need to investigate the long double conversion failures on MacOS |
Also simplifies the H5MM package, replacing most H5MM replacement functions with defines to C standard library functions