-
Notifications
You must be signed in to change notification settings - Fork 17
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
Cleanup and fix some memory leaks #43
Conversation
for compatibility with modern compiler
@ntamas It might be a good idea to check the Clang Analyzer results on this package. Do it like this:
The use Product -> Analyze. There are "Left operand of Then there are some null pointer dereference warnings in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
I'll do the static analysis and then check whether I can fix any more of those warnings. Error checks with OpenMP are indeed problematic -- you cannot bail out in the middle of an OpenMP section but you need to coordinate until you get back to the main thread and then you can do the cleanup there. I'll look into this. |
Okay, FYI, I went through the static analysis results.
|
I wouldn't spend much time on this right now, as the errors are probably OOM only, and if that happens, the program is likely to crash anyway. At one point we'll need to figure out a proper way to do error checks from within OpenMP code and start adding OpenMP to igraph itself. That will require careful thought and it's likely to be a lot of work. |
This carries over all changes that were made in igraph, not just igraph/igraph#2433 There's a fix for
void
parameter lists.Some of the headers I removed on the igraph side were actually necessary here (for
rand()
).I tested that this compiled with
-std=gnu89
and-std=c99
, but it does not compile with-std=c89
. That's because plfit uses many functions which were present as a GNU extension for a long time, but only became standard in C99.