Re: [PATCH] parse_object: clear "parsed" when freeing buffers

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 24, 2013, 16:51 UTC
Message-ID: <7vzjzycyne.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130124063259.GA610@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 20 quoted lines

> The ensure_commit_buffer function could look something like:
>
>   int ensure_commit_buffer(struct commit *item)
>   {
>           enum object_type type;
>           unsigned long size;
>
>           if (!item)
>                   return -1;
>           if (!item->object.parsed)
>                   return parse_commit(item);
>           if (item->buffer)
>                   return 0;
>
>           item->buffer = read_sha1_file(item->object.sha1, &type, &size);
>           if (!item->buffer)
>                   return error("Could not read %s",
>                                sha1_to_hex(item->object.sha1);
>           return 0;
>   }

The more important issue is when to release them.

Only the subcommands that know that what they are doing do not need to access commit->buffer and they operate solely on parsed data are suppposed to flip the save_commit_buffer bit off to reclaim the memory early, so new callers that violate that assumption may want to call ensure_*, but if they did so when somebody cares about the memory pressure enough to turn the bit off need to free the buffer themselves after they use it. Otherwise the resulting program may still be "correct" but we will end up keeping buffer that nobody will use, just in case it is asked again.