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

> 
> 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.