Re: [PATCH v3] pack-bitmap: remove checks before bitmap_free

[Junio C Hamano <gitster@pobox.com>]

lidongyan <502024330056@smail.nju.edu.cn> writes:

> No, this test case should only fail when ’SANITIZE_LEAK’ is set. I heard
> that other developer call this type of test as prereq. So only when git is
> compiled with `-fsanitize=address` and `export ASAN_OPTION=detect_leaks=1`
> and without changes as
>
> - if (!cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap))
> - bitmap_free(roots_bitmap);
> + cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap);
> + bitmap_free(roots_bitmap);
>
> This test case would fail.

If the test tickles the code path that used to be broken (and
corrected by the patch), temporarily reverting only the code changes
to pack-bitmap.c and then this test (under leak sanitizer, of
course) should have failed.  And if the test passed with such an
experiment, you would have noticed that something is wrong.

But you didn't notice it and sent the patch, so I'd assume that you
saw such a test still failed.  IOW, with "export" forgotten in the
test, the original (unfixed) code still leaked, without using the
bitmap traversal, right?

Which was where my question came from.

Or perhaps you didn't do that "is my test really tickling the bug I
fixed and makes the original code without my fix fail?" test?  Which
also explains why lack of "export" was not noticed.