Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Hey @eschnett (This is responding to #1025 (comment) as well) |
for more information, see https://pre-commit.ci
|
A high-level design question for a C interface is how dynamically allocated structures or arrays should be handled. I am currently using this approach: char* get_stuff();
size_t get_stuffSize();where Another approach that could be used would be size_t get_stuff(char* ptr, size_t provided_size);where the caller needs to allocate the memory. which writes the necessary size into This choice is on one hand inconsequential – it just specifies how memory is allocated or freed – but is on the other hand pervasive throughout the API. |
for more information, see https://pre-commit.ci
I think that your current approach is fine. openPMD-api is not so low-level that memory management needs to be done by the user IMO. I'd suggest that we add either something like Is this PR ready for review? Give me a note if you need help with anything (like CMake integration of the tests). |
|
|
||
| openPMD_Series *openPMD_Series_new(); | ||
|
|
||
| #if openPMD_HAVE_MPI |
There was a problem hiding this comment.
For the C++ API we're planning to split headers into parallel and non-parallel headers which would simplify shipping openPMD-api since you don't need separate MPI and non-MPI builds.
It would probably be good to start out with such a separation in the C bindings from the beginning already.
There was a problem hiding this comment.
I'll think about it. My main use case is calling openPMD_api from Julia, and there we always have MPI available.
|
@franzpoeschel The code is not yet ready for review, some functions are still missing. |
No description provided.