On Fri, Jan 25, 2013 at 7:55 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Jan 24, 2013 at 03:56:23PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> writes:
>>
>> > ... (e.g., how should "log" know that a submodule diff might later want
>> > to see the same entry? Should we optimistically free and then make it
>> > easier for the later user to reliably ensure the buffer is primed? Or
>> > should we err on the side of keeping it in place?).
>>
>> My knee-jerk reaction is that we should consider that commit->buffer
>> belongs to the revision traversal machinery. Any other uses bolted
>> on later can borrow it if buffer still exists (I do not think pretty
>> code rewrites the buffer contents in place in any way), or they can
>> ask read_sha1_file() to read it themselves and free when they are
>> done.
>
> Yeah, that is probably the sanest way forward. It at least leaves
> ownership in one place, and everybody else is an opportunistic consumer.
> We do need to annotate each user site though with something like the
> "ensure" function I mentioned.
>
> If they are not owners, then the better pattern is probably something
> like:
You probably should rename "buffer" (to _buffer or something) and let
the compiler catches all the places commit->buffer illegally used.
>
> /*
> * Get the commit buffer, either opportunistically using
> * the cached version attached to the commit object, or loading it
> * from disk if necessary.
> */
> const char *use_commit_buffer(struct commit *c)
> {
> enum object_type type;
> unsigned long size;
>
> if (c->buffer)
> return c->buffer;
> /*
> * XXX check type == OBJ_COMMIT?
> * XXX die() on NULL as a convenience to callers?
> */
> return read_sha1_file(c->object.sha1, &type, &size);
> }
>
> void unuse_commit_buffer(const char *buf, struct commit *c)
> {
> /*
> * If we used the cached copy attached to the commit,
> * we don't want to free it; it's not our responsibility.
> */
> if (buf == c->buffer)
> return;
>
> free((char *)buf);
> }
>
> I suspect that putting a use/unuse pair inside format_commit_message
> would handle most cases.
>
> -Peff
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html