[PATCH v2 0/2] pack-bitmap: remove checks before bitmap_free
- From
Lidong Yan via GitGitGadget <gitgitgadget@gmail.com>
- Date
- May 30, 2025, 18:14 UTC
- Message-ID
- <pull.1977.v2.git.git.1748628846.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1977.git.git.1748149783383.gitgitgadget@gmail.com>
In pack-bitmap.c:find_boundary_objects, remove cascade success check and always free roots_bitmap afterward to make static analysis tool works better.
Lidong Yan (2): pack-bitmap: remove checks before bitmap_free t5333: test memory leak when use pseudo-merge in boundary traversal
pack-bitmap.c | 4 ++-- t/t5333-pseudo-merge-bitmaps.sh | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-)
base-commit: 845c48a16a7f7b2c44d8cb137b16a4a1f0140229 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1977%2Fbrandb97%2Fremove-check-before-bitmap-free-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1977/brandb97/remove-check-before-bitmap-free-v2 Pull-Request: https://github.com/git/git/pull/1977
Range-diff vs v1:
1: 19677bcbc3d ! 1: d7b7a0e29ec pack-bitmap: remove checks before bitmap_free
@@ Commit message
pack-bitmap: remove checks before bitmap_free
In pack-bitmap.c:find_boundary_objects, we build a roots_bitmap and
- cascade it to cb.base. However, I’m wondering why we only free
- roots_bitmap when the cascade succeeds. It seems we could safely remove
- this check and always free roots_bitmap afterward, which might provide
- some performance benefits.
+ cascade it to cb.base. Only when cascade failed, roots_bitmap is
+ freed otherwise it leaks. Since cascade_pseudo_merges_1() only use
+ roots_bitmap as a mutable reference not takes roots_bitmap's ownership
+ we'd better remove `if(cascade_pseudo_merges_1)` and frees roots_bitmap
+ anyway.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
-: ----------- > 2: 56b24d681cb t5333: test memory leak when use pseudo-merge in boundary traversal-- gitgitgadget