On Tue, Aug 29, 2017 at 06:51:52PM +0100, Lars Schneider wrote:
> I set $TOOL_OPTIONS in valgrind.sh: to
> '--leak-check=full --errors-for-leak-kinds=definite'
>
> ... but I also had to adjust t/test-lib-functions.sh:test_create_repo
> as I ran into the error "cannot run git init -- have you built things yet?".
Yeah, this is a general problem with run-time analyzers. If we're not
mostly error free than the setup code often breaks before you even get
to the interesting part of the test.
It looks like the config code has a minor-ish leak. Patch to follow.
> 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. :)
-Peff
