Fix: improve storage tool checks, disable default allocator check, fix mwcsys_executil #289
Conversation
678098
force-pushed
the
240514_fix_AIX_build
branch
from
53bc665
to
77cf1bf
Compare
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
678098
force-pushed
the
240514_fix_AIX_build
branch
from
ee54b13
to
9c70a6b
Compare
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
678098
changed the title
| @@ -130,7 +130,7 @@ static void test2_executeSystemFailure() | |||
|
|
|||
| PVV("Testing abnormal exit of the command"); | |||
| rc = mwcsys::ExecUtil::execute(&output, | |||
| "python -c 'import os,signal; " | |||
| "python3 -c 'import os,signal; " | |||
There was a problem hiding this comment.
We cannot use implicit python2 here, python3 alias might work on most frequently known systems
We still might be in trouble if we don't have python3 alias on a system but have only a specific version like python3.11. For me, it's worth trying to test
Ideally, the test should use a tool with more rigid (stable) name, but it requires more time to fix
There was a problem hiding this comment.
I'm fine with this test. If it fails, we can use use python3.11 etc, but I'd rather not tie ourselves to a specific Python version if we can help it.
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
| s_ignoreCheckDefAlloc = true; | ||
| // Disable default allocator check for this test until we can debug | ||
| // it on AIX/Solaris | ||
| #endif |
There was a problem hiding this comment.
I am not sure what code do allocations wrongly, it is difficult to find without means to build and test it on AIX/Solaris. I reviewed the storage tool and improved many things related to allocations, but maybe we should just disable this check until we debug it on the needed platforms
There was a problem hiding this comment.
This makes sense to me. It's at least better to do this test by default on Linux, which will give us at least some early indications with GH Actions.
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
| s_ignoreCheckDefAlloc = true; | ||
| // Disable default allocator check for this test until we can debug | ||
| // it on AIX/Solaris | ||
| #endif |
There was a problem hiding this comment.
This makes sense to me. It's at least better to do this test by default on Linux, which will give us at least some early indications with GH Actions.
| @@ -130,7 +130,7 @@ static void test2_executeSystemFailure() | |||
|
|
|||
| PVV("Testing abnormal exit of the command"); | |||
| rc = mwcsys::ExecUtil::execute(&output, | |||
| "python -c 'import os,signal; " | |||
| "python3 -c 'import os,signal; " | |||
There was a problem hiding this comment.
I'm fine with this test. If it fails, we can use use python3.11 etc, but I'd rather not tie ourselves to a specific Python version if we can help it.
| @@ -102,8 +102,8 @@ class JournalFile { | |||
| // Number of records. | |||
| MappedFileDescriptor d_mfd; | |||
| // Mapped file descriptor. | |||
| char* d_mem_p; | |||
| // Pointer to allocated memory for journal file. | |||
| bsl::vector<char> d_mem; | |||
There was a problem hiding this comment.
Vector is fine (exception safe!). I'm surprised there isn't a clear BDE component for this use case, but I couldn't find one with a quick search.
There was a problem hiding this comment.
I believe allocating data buffers via vector is a common practice for safe (not a joke, but still sounds funny) C++ programming, just as using smart pointers and other RAII things
There was a problem hiding this comment.
From the past days, we used levmar lib for optimizations in C++ code, with API like this:
int dlevmar_dif(
void (*func)(double *p, double *hx, int m, int n, void *adata)
And we were really happy to use vector buffers on top of this
Storage tool: