]> Kevux Git Server - rit/commit
unpack_loose_rest(): never clean up zstream
authorJeff King <peff@peff.net>
Tue, 25 Feb 2025 06:33:12 +0000 (01:33 -0500)
committerJunio C Hamano <gitster@pobox.com>
Tue, 25 Feb 2025 18:25:49 +0000 (10:25 -0800)
commit84b5c1a099e6df35f4b54d651b425a894513e62b
tree0afc4fd1c30bc596b10385a3296e2455755e8785
parent9929a6791703c96e5f613cc3b52f4f9e16baa49c
unpack_loose_rest(): never clean up zstream

The unpack_loose_rest() function has funny ownership semantics: we pass
in a z_stream opened by the caller, but then only _sometimes_ close it.

This oddity has developed over time. When the function was originally
split out in 5180cacc20 (Split up unpack_sha1_file() some more,
2005-06-02), it always called inflateEnd() to clean up the stream
(though nowadays it is a git_zstream and we call git_inflate_end()).

But in 7efbff7531 (unpack_sha1_file(): detect corrupt loose object
files., 2007-03-05) we added error code paths which don't close the
stream. This makes some sense, as we'd still look at parts of the stream
struct to decide which error to show (though I am not sure in practice
if inflateEnd() even touches those fields).

This subtlety makes it hard to know when the caller has to clean up the
stream and when it does not. That led to the leak fixed by aa9ef614dc
(object-file: fix memory leak when reading corrupted headers,
2024-08-14).

Let's instead always leave the stream intact, forcing the caller to
clean it up. You might think that would create more work for the
callers, but it actually ends up simplifying them, since they can put
the call to git_inflate_end() in the common cleanup code path.

Two things to note, though:

  - The check_stream_oid() function is used as a replacement for
    unpack_loose_rest() in read_loose_object() to read blobs. It
    inherited the same funny semantics, and we should fix it here, too
    (to keep the cleanup in read_loose_object() consistent).

  - In read_loose_object() we need a second "out" label, as we can jump
    to the existing label before opening the stream at all (and since
    the struct is opaque, there is no way to if it was initialized or
    not, so we must not call git_inflate_end() in that case).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
object-file.c