Added a check to ensure memory tools is working#16
Conversation
|
/cc @serge-nikulin |
|
CI:
|
|
|
||
| #include "osrf_testing_tools_cpp/memory_tools/is_working.hpp" | ||
|
|
||
| #include <string> |
There was a problem hiding this comment.
Nit: #include <cstdlib> for std::malloc/std::free.
| memcpy(some_memory, str.c_str(), str.length()); | ||
| memcpy((void *)side_effect, some_memory, str.length()); | ||
| std::free(some_memory); | ||
| osrf_testing_tools_cpp::memory_tools::guaranteed_malloc(str); |
There was a problem hiding this comment.
FYI, the same pattern exists in https://github.com/osrf/osrf_testing_tools_cpp/blob/master/osrf_testing_tools_cpp/test/memory_tools/test_memory_tools.cpp#L152 , so we'll want to call the new function there as well.
There was a problem hiding this comment.
You're right, I'll update it to modify that instance as well, thanks!
| /// Get the current on_realloc callback if set, otherwise null. | ||
| OSRF_TESTING_TOOLS_CPP_MEMORY_TOOLS_PUBLIC | ||
| AnyMemoryToolsCallback | ||
| get_on_realloc(); |
There was a problem hiding this comment.
Given that we don't actually use get_on_realloc/get_on_calloc/get_on_free, should we bother exposing them? I generally like to only expose the things that I'm using, to keep the API smaller.
There was a problem hiding this comment.
I considered the same thing, but I errored on the side of symmetry. So unless there's some other reason I'd opt to not touch it again.
There was a problem hiding this comment.
That's fine, we can leave it.
Signed-off-by: William Woodall <william@osrfoundation.org>
|
Thanks for the review, I think I addressed the feedback, can you please re-review when you get some time? |
|
New CI:
|
2 participants
Fixes #14