Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

From: Martin Ågren <martin.agren@gmail.com>
Date: Aug 29, 2017, 19:22 UTC
Message-ID: <CAN0heSomjqiSStq3eqGgCe21b0Pr0gbAQLL-2EEB6Zfnjj+Mjg@mail.gmail.com>
In-Reply-To: <20170829185341.s3xlsx4uym7lcluc@sigill.intra.peff.net>

On 29 August 2017 at 20:53, Jeff King <peff@peff.net> wrote:

Show 28 quoted lines

> On Tue, Aug 29, 2017 at 06:51:52PM +0100, Lars Schneider wrote:
>> What if we run a few selected tests with valgrind and count all files that
>> valgrind mentions (a single leak has multiple file mentions because of
>> the stack trace and other leak indicators). We record these counts and let
>> TravisCI scream if one of the numbers increases.
>>
>> I wonder how stable/fragile such a metric would be as a simple refactoring
>> could easily change these numbers. Below I ran valgrind on t5510 before and
>> after Martin's patch. The diff below clearly shows the pkt-line leak.
>>
>> Would it make sense to pursue something like this in TravisCI to avoid
>> "pkt-line" kind of leaks in the future?
>
> I don't think that would work, because simply adding new tests would
> bump the leak count, without the code actually growing worse.
>
> But think about your strategy for a moment: what you're really trying to
> do is say "these existing leaks are OK because they are too many for us
> to count, but we want to make sure we don't add _new_ ones". We already
> have two techniques for distinguishing old ones from new ones:
>
>   1. Mark existing ones with valgrind suppressions so they do not warn
>      at all.
>
>   2. Fix them, so that the "existing" count drops to zero.
>
> Option (2), of course, has the added side effect that it's actually
> fixing potential problems. :)

One idea would be to report that "t1234 used to complete without leaks, but now there is a leak". Of course, the leak could be triggered by some "irrelevant" setup step that was just added and not by the functionality that is actually tested. But maybe then the reaction might be "oh, let's fix this one leak so that this test is leak-free again". If that then also reduces the leakiness of some other tests, good. That might help against the "there are too many leaks, where should I start?"-effect.

Or similarly "This stack-trace was reported. I haven't seen it before." It could be a refactoring, it could be an old leak that hasn't been triggered in that way before, or it could be a new leak.

But to be honest, I think that only works -- in a psychological sense -- once the number of leaks is small enough, however much that is.

One take-away for me from this thread is that memory-leak-plugging seems to be appreciated, i.e., it's not just an academic problem. I think I'll look into it some more, i.e., slowly pursue option 2. :-) That might help give some insights on how to identify interesting leaks.

Martin