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 memleak: free the job object returned. #1322

Merged
merged 1 commit into from Feb 5, 2018

Conversation

Projects
None yet
4 participants
@nikhil-jain
Copy link
Contributor

nikhil-jain commented Jan 31, 2018

I think the returned object should be caught and freed. Valgrind reported this leak, so likely legit.

Fix memleak: free the job object returned.
Change-Id: If0701b37b095317751868e0c481aa6560bb2564e
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 31, 2018

Codecov Report

Merging #1322 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1322      +/-   ##
==========================================
- Coverage    78.2%   78.17%   -0.03%     
==========================================
  Files         156      156              
  Lines       28269    28270       +1     
==========================================
- Hits        22107    22101       -6     
- Misses       6162     6169       +7
Impacted Files Coverage Δ
src/common/libjsc/jstatctl.c 78.42% <100%> (+0.02%) ⬆️
src/modules/connector-local/local.c 72.95% <0%> (-1.44%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/common/libflux/message.c 81.36% <0%> (-0.12%) ⬇️
src/broker/overlay.c 74.2% <0%> (ø) ⬆️
src/cmd/flux-module.c 84.75% <0%> (+0.3%) ⬆️
src/common/libutil/dirwalk.c 94.28% <0%> (+0.71%) ⬆️
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 31, 2018

Coverage Status

Coverage decreased (-0.02%) to 78.506% when pulling 5a82e29 on nikhil-jain:memleak-fix-rebased into 7969ee6 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Jan 31, 2018

Yes that looks right to me too. Thanks!

@garlick garlick merged commit 49b2916 into flux-framework:master Feb 5, 2018

4 checks passed

codecov/patch 100% of diff hit (target 78.2%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +21.79% compared to 7969ee6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 78.506%
Details

@nikhil-jain nikhil-jain referenced this pull request Feb 5, 2018

Closed

Memory leak in Jtostr #1325

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.