Re: [PATCH] pack-bitmap: add loading corrupt bitmap_index test

From: lidongyan <502024330056@smail.nju.edu.cn>
Date: May 19, 2025, 06:44 UTC
Message-ID: <80BCF957-002B-4532-8E3D-8CAB45AC0349@smail.nju.edu.cn>
In-Reply-To: <aCrJcK6ml4r4S-mF@pks.im>

2025年5月19日 14:02，Patrick Steinhardt <ps@pks.im> 写道：

> Okay. We _can_ do that now, but the patch doesn't explain why we
> _should_.

The main purpose of this patch is to provide a test case to check whether
 a memory leak occurs when loading a corrupt index file as requested here
https://lore.kernel.org/git/20250514180325.GB2196784@coredump.intra.peff.net.
A potential memory leak is mentioned in this patch here https://lore.kernel.org/git/pull.1962.git.git.1747052530271.gitgitgadget@gmail.com/.

Show 11 quoted lines

> Does all of this logic really have to be part of "pack-bitmap.c"? It
> would generally preferable to not have our production logic be cluttered
> with test logic. Sometimes we don't have a better way to do this, but
> you should explain why we cannot host the logic elsewhere in that case.
> 
> My proposal would be to either move the logic into "test-bitmap.c", or
> to even better to write a unit test in "t/unit-tests/". After all, we
> expect that the code should fail gracefully, so a unit test might be a
> good fit after all.
> 
> Patrick

I found that the header size of an index file depends only on the type of hash algorithm. To trigger the condition for the memory leak, I need to corrupt a few bytes right after the index file header size. It's more convenient to implement this functionality in pack-bitmap.c. However, I think I can place the test itself under t/unit-tests/.