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

Fix a few memory issues (3x heap-buffer-overflow, race condition?) #4

Closed
wants to merge 0 commits into from
Closed

Conversation

LebedevRI
Copy link
Contributor

Hello.

I'm not sure if GitHub PR is the prefered contribution method, but i would prefer not to touch SF :(

Recently i have been using this program a lot, so i though i may aswell try to contribute something back :)
So here are a few fixes for some of the issues i was able to reproduce and fix.

Please review last commit (a_background_update_status) with care, i'm not sure why that variable was static.

PS: i have tested a patch in bug 121, and so far it seems to fix that issue. I'm looking forward to seeing it merged.

Please pull.
Thanks!

@rnorris
Copy link
Collaborator

rnorris commented Aug 31, 2015

I've applied the first two changes.

"Please review last commit (a_background_update_status) with care, i'm not sure why that variable was static."

Since this code comes from the first public version of Viking, I can only assume the original author wrote it this way under the impression it would be 'faster' by avoiding unnecessary memory allocations.

However these gains are very marginal since this operation doesn't happen thousands of times and potentially open to race conditions when accessed via multiple threads.

If you want I can apply the commit and remove the 'NOTE: ... do not understand....' text in the comment?

@LebedevRI
Copy link
Contributor Author

@rnorris
Updated. + 1 more fix.

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

2 participants