Message ID | pull.1177.git.git.1640927702745.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | t4126: this test does not pass SANITIZE_LEAK; quit claiming it does | expand |
Hi Elijah, On Fri, 31 Dec 2021, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > t4126: this test does not pass SANITIZE_LEAK; quit claiming it does > > My builds in several different topics keep running into this, and I'm > sure it's a false positive -- I didn't change anything affecting this > test. I've just been ignoring it and submitting anyway, and I suspect > others are too. So that's what is causing this. Thank you for chasing it down, it has been on my TO-DO list e.g. due to https://github.com/git-for-windows/git/runs/4622790431?check_suite_focus=true#step:5:146 Given that it points to a leak in `cmd_format_patch()` (oh no, we're _LEAKING_ some _MEMORY_ in a built-in, how did that ever happen, we must free it before... *checks notes* ... wait, we're quitting immediately after this function anyway? </sarcasm>) I wonder whether the disruption caused by `linux-leaks` should really be considered worth the benefit. Anyway. Thank you for the patch! Ciao, Dscho
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Elijah Newren <newren@gmail.com> > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > t4126: this test does not pass SANITIZE_LEAK; quit claiming it does Is this continuation of [*1*]? I think it will go away when dee839a2 (format-patch: mark rev_info with UNLEAK, 2021-12-16) is merged. [Reference] *1* https://lore.kernel.org/git/xmqqee6dz5s9.fsf@gitster.g/
On Sat, Jan 1, 2022 at 5:21 PM Junio C Hamano <gitster@pobox.com> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Elijah Newren <newren@gmail.com> > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > t4126: this test does not pass SANITIZE_LEAK; quit claiming it does > > Is this continuation of [*1*]? Ah, yes it is. > I think it will go away when dee839a2 (format-patch: mark > rev_info with UNLEAK, 2021-12-16) is merged. Can we fast track that patch? The patch looks good to me. In general, though, I would think the simpler fix of just removing the TEST_PASSES_SANITIZE_LEAK=true would be safer and could be fast tracked. I'm sorry I missed these potential problems cropping up when I reviewed the earlier series (which added these flags to a bunch of scripts). I just didn't foresee these consequences.
diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh index 82284d2f45d..ebbac79f20e 100755 --- a/t/t4126-apply-empty.sh +++ b/t/t4126-apply-empty.sh @@ -3,7 +3,6 @@ test_description='apply empty' -TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup '