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

Rename error.h to osqp_error.h to avoid OS collisions #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AustinSchuh
Copy link

On linux, error.h exists. Depending on how your toolchain and compiler
is configured, the compiler can pick the system error.h instead.
This results in some pretty esoteric errors which are hard to debug.
Instead, prefix this file with osqp_ to avoid the collision.

On linux, error.h exists.  Depending on how your toolchain and compiler
is configured, the compiler can pick the system error.h instead.
This results in some pretty esoteric errors which are hard to debug.
Instead, prefix this file with osqp_ to avoid the collision.
@CLAassistant
Copy link

CLAassistant commented Feb 19, 2022

CLA assistant check
All committers have signed the CLA.

@imciner2
Copy link
Member

Thank you for proposing this. Please note that our next planned release is being developed on the develop-1.0 branch, not the master branch, so please retarget the PR against that branch.

I am also somewhat confused what you mean by the compiler getting confused between the OSQP error.h and the system error.h. The header is specified as a local/non-system header in the include statements (through the use of quotes), and the include path where it is located should be placed by CMake earlier in the compilation command than the default system path (so it should take precedence). Can you be more precise about the system/compilers that are confusing the two headers please?

@mhubii
Copy link

mhubii commented Mar 4, 2022

It is interesting to mention that these errors would be resolved by #401

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.

None yet

4 participants