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

fixes mem leak in xlib joysticks #1251

Merged
merged 1 commit into from May 29, 2018
Merged

fixes mem leak in xlib joysticks #1251

merged 1 commit into from May 29, 2018

Conversation

RemoveRusky
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented May 28, 2018

Codecov Report

Merging #1251 into master will decrease coverage by 0.02%.
The diff coverage is 5.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1251      +/-   ##
==========================================
- Coverage   10.81%   10.78%   -0.03%     
==========================================
  Files         158      159       +1     
  Lines       17079    17082       +3     
==========================================
- Hits         1847     1843       -4     
- Misses      15232    15239       +7
Impacted Files Coverage Δ
...NIGMAsystem/SHELL/Platforms/xlib/LINUXjoystick.cpp 10.92% <5.55%> (-3.49%) ⬇️
ENIGMAsystem/SHELL/Universal_System/make_unique.h 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6001a6...90b1081. Read the comment docs.

@RemoveRusky
Copy link
Contributor Author

Ready for merge

}
};
static vector<e_joystick*> joysticks;

static vector<std::unique_ptr<e_joystick>> joysticks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing a private container of unique_ptr is pretty tacky; what was wrong with freeing the pointer ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we never did

Copy link
Member

@JoshDreamland JoshDreamland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should continue to do so in the future, but this is fine for a stopgap. If you feel like driving the checkin/installation process, I'll commit the framework for extension hooks to a new branch. Bug me when it's something you want to deal with.

@JoshDreamland JoshDreamland merged commit db85197 into master May 29, 2018
@JoshDreamland JoshDreamland deleted the pleasureRod branch May 29, 2018 02:15
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

3 participants