From: lidongyan <502024330056@smail.nju.edu.cn> Date: Mon, 19 May 2025 06:44:22 GMT Subject: Re: [PATCH] pack-bitmap: add loading corrupt bitmap_index test Message-ID: <80BCF957-002B-4532-8E3D-8CAB45AC0349@smail.nju.edu.cn> In-Reply-To: 2025年5月19日 14:02，Patrick Steinhardt 写道： > 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/. > 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/.