Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use c_free for memory that has been allocated with c_malloc and c_calloc #34

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

traversaro
Copy link
Contributor

I was compiling the osqp_mex function against an already compiled osqp version (similar to what was described in #27), and my MATLAB was crashing because quantities that were allocated with c_malloc and c_calloc were then freed with mxFree.

While the direct use of c_malloc and c_calloc can be converted to the corresponding mx** allocator by defining MATLAB for the osqp_mex compilation unit, this was not possible for the allocation done via csc_matrix, as fhat function is compiled as part of OSQP and whose definition can't be changed.

Changing to use c_free for freeing all the memory that was allocated with c_malloc and c_calloc fixes the problem, and works as expected even if MATLAB is defined, as in the case of the official build procedure for the MATLAB bindings.

The memory was allocated indirectly via copyToCfloatVector and copyToCfloatVector functions
@traversaro
Copy link
Contributor Author

I updated the PR with a new commit ( traversaro@09faf01 ) as it turns out that there were other mismatched calls (c_malloc/mxFree), as reported in ami-iit/osqp-matlab-cmake-buildsystem#5 .
With this changes, I was able to run the complete test suite (without codegen tests).

@imciner2
Copy link
Member

Thanks, I don't think we had ever tried using the mex file with a version of OSQP that wasn't built for MATLAB (since we always staticly link the interface in this repo in its CMake.).

@imciner2 imciner2 merged commit b98f166 into osqp:master Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants