[PATCH 14/20] tempfile: release deactivated strbufs instead of resetting
- From
Jeff King <peff@peff.net>
- Date
- Sep 5, 2017, 12:14 UTC
- Message-ID
- <20170905121456.x4gwz3kuvvygpt55@sigill.intra.peff.net>
- In-Reply-To
- <20170905121353.62zg3mtextmq5zrs@sigill.intra.peff.net>
When a tempfile is deactivated, we reset its strbuf to the empty string, which means we hold onto the memory for later reuse.
Since we'd like to move to a system where tempfile structs can actually be freed, deactivating one should drop all resources it is currently using. And thus "release" rather than "reset" is the appropriate function to call.
In theory the reset may have saved a malloc() when a tempfile (or a lockfile) is reused multiple times. But in practice this happened rarely. Most of our tempfiles are single-use, since in cases where we might actually use many (like ref locking) we xcalloc() a fresh one for each ref. In fact, we leak those locks (to appease the rule that tempfile storage can never be freed). Which means that using reset is actively hurting us: instead of leaking just the tempfile struct, we're leaking the extra heap chunk for the filename, too.
Signed-off-by: Jeff King <peff@peff.net> --- tempfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tempfile.c b/tempfile.c index 3348ad59dd..e655e28477 100644 --- a/tempfile.c +++ b/tempfile.c @@ -128,7 +128,7 @@ static void activate_tempfile(struct tempfile *tempfile) static void deactivate_tempfile(struct tempfile *tempfile) { tempfile->active = 0; - strbuf_reset(&tempfile->filename); + strbuf_release(&tempfile->filename); } /* Make sure errno contains a meaningful value on error */
-- 2.14.1.721.gc5bc1565f1