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 more leaks detected by ASAN #10219

Merged
merged 2 commits into from
Apr 22, 2020
Merged

Conversation

dioni21
Copy link
Contributor

@dioni21 dioni21 commented Apr 17, 2020

Motivation and Context

Found some Leaks using ASAN compilation

Description

This commit fixes a bunch of missing free() calls created in a10d50f

How Has This Been Tested?

Build and run. Message is gone.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

This commit fixes a bunch of missing free() calls in a10d50f

Signed-off-by: João Carlos Mendes Luís <jonny@jonny.eng.br>
@dioni21
Copy link
Contributor Author

dioni21 commented Apr 17, 2020

@behlendorf could you please take a look at those test failures? I thought these changes were extremely simple, and the most intriguing is that only Ubuntu 18.04 failed.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks. I agree, the failures look unrelated. We do have a couple false positives which can occur, let's see if we see these same failures again after updating this PR for the minor style issue.

lib/libzfs/libzfs_dataset.c Outdated Show resolved Hide resolved
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Apr 17, 2020
@dioni21 dioni21 requested a review from behlendorf April 17, 2020 19:00
Noted by @behlendorf

Signed-off-by: João Carlos Mendes Luís <jonny@jonny.eng.br>
@dioni21
Copy link
Contributor Author

dioni21 commented Apr 17, 2020

Good addition that STYLE check, BTW!

@codecov-io
Copy link

codecov-io commented Apr 18, 2020

Codecov Report

Merging #10219 into master will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10219      +/-   ##
==========================================
+ Coverage   79.38%   79.55%   +0.17%     
==========================================
  Files         388      388              
  Lines      123392   123396       +4     
==========================================
+ Hits        97953    98171     +218     
+ Misses      25439    25225     -214     
Flag Coverage Δ
#kernel 80.01% <ø> (+0.08%) ⬆️
#user 66.27% <100.00%> (+0.77%) ⬆️
Impacted Files Coverage Δ
lib/libzfs/libzfs_dataset.c 76.48% <100.00%> (+0.04%) ⬆️
module/os/linux/spl/spl-zlib.c 55.35% <0.00%> (-28.58%) ⬇️
cmd/zvol_id/zvol_id_main.c 76.31% <0.00%> (-5.27%) ⬇️
module/zfs/dsl_synctask.c 92.40% <0.00%> (-2.54%) ⬇️
cmd/zed/agents/fmd_api.c 88.61% <0.00%> (-1.78%) ⬇️
module/os/linux/zfs/zfs_dir.c 82.91% <0.00%> (-0.69%) ⬇️
module/zfs/vdev_indirect.c 74.00% <0.00%> (-0.67%) ⬇️
cmd/zed/agents/zfs_mod.c 77.55% <0.00%> (-0.67%) ⬇️
cmd/zed/agents/zfs_diagnosis.c 78.42% <0.00%> (-0.30%) ⬇️
lib/libzfs/libzfs_mount.c 84.21% <0.00%> (-0.23%) ⬇️
... and 55 more

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 a7929f3...d651319. Read the comment docs.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 20, 2020
@behlendorf behlendorf merged commit 70e5ad3 into openzfs:master Apr 22, 2020
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
This commit fixes a bunch of missing free() calls in a10d50f

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: João Carlos Mendes Luís <jonny@jonny.eng.br>
Closes openzfs#10219 
(cherry picked from commit 70e5ad3)
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
This commit fixes a bunch of missing free() calls in a10d50f

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: João Carlos Mendes Luís <jonny@jonny.eng.br>
Closes openzfs#10219 
(cherry picked from commit 70e5ad3)
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
This commit fixes a bunch of missing free() calls in a10d50f

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: João Carlos Mendes Luís <jonny@jonny.eng.br>
Closes openzfs#10219
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants