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 memory leak in traffic_class_test and improves traffic class deletion behavior. #199

Merged
merged 2 commits into from
Dec 22, 2016

Conversation

barath
Copy link
Contributor

@barath barath commented Dec 22, 2016

No description provided.

@barath barath requested a review from melvinw December 22, 2016 07:08
@codecov-io
Copy link

codecov-io commented Dec 22, 2016

Current coverage is 62.83% (diff: 100%)

Merging #199 into master will increase coverage by 0.17%

@@             master       #199   diff @@
==========================================
  Files            46         46          
  Lines          2129       2139    +10   
  Methods         278        278          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1334       1344    +10   
  Misses          795        795          
  Partials          0          0          

Powered by Codecov. Last update afaaba5...8e4e3e7

@barath barath requested review from sangjinhan and changlan and removed request for melvinw December 22, 2016 17:53
@barath
Copy link
Contributor Author

barath commented Dec 22, 2016

@changlan This should fix the issue you opened.

@barath barath requested review from melvinw and removed request for sangjinhan and changlan December 22, 2016 19:15
TrafficClass *c = CT("leaf", {LEAF});
ASSERT_TRUE(c != nullptr);
std::unique_ptr<TrafficClass> c(CT("leaf", {LEAF}));
ASSERT_TRUE(c.get() != nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit. should use ASSERT_NE here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Also replaced other similar assertions with ASSERT_NE.

@melvinw
Copy link
Contributor

melvinw commented Dec 22, 2016

Looks good! Will merge once CI finishes

@melvinw melvinw merged commit e214582 into master Dec 22, 2016
@melvinw
Copy link
Contributor

melvinw commented Dec 22, 2016

Thanks!
restofcat

@barath barath deleted the tc-memory-leak-fix branch December 22, 2016 21:39
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.

3 participants