Skip to content

Fix various memory leaks

Patrick Steinhardt requested to merge pks-t-mark-leak-free-tests into master

My mind had a couple of minutes where it was roaming, and of course it immediately searched for and chased down the next rabbit hole. The result is this patch series which fixes a bunch of leaks all over the place. There isn't really any structure to the leaks that I did fix -- it's mostly things that I stumbled over. In the end, this series makes another 56 test suites pass with leak checking enabled, 13 of which have already been passing without any changes.

While most things are unstructured, there are two topics that stand out:

  • Patches 5 to 12 address a shortcoming of our config API. Both git_config_string() and git_config_pathname() have a const char ** out parameter, but they do in fact transfer memory ownership to the caller. This resulted in a bunch of memory leaks all over the place.

    These patches thus refactor a bunch of code and then ultimately switch the out parameter to become a char *

  • Patches 16 to 20 have the goal of making git-mv(1) memory leak free. I had a very hard time understanding how it tracks memory. I think this wasn't only me, or otherwise there wouldn't be calls to UNLEAK() in there. In any case, I decided to rewrite the arrays to use a struct strvec, which makes tracking and releasing of memory a ton easier.

    It does come at the cost of more allocations because we may now duplicate strings that we didn't before. But I think the tradeoff is worth it because the number of strings we may now duplicate is bounded by the number of command line arguments anyway.

Edited by Patrick Steinhardt

Merge request reports