Subject: Re: [PATCH v3 1/4] replace: add --graft option

From: Junio C Hamano <[email-masked]>
Date: Mon, 09 Jun 2014 09:43:07 -0700

Jeff King <[email-masked]> writes:

> I think it would make sense to actually take this one step further,
> though, and move commit->buffer into the slab, as well. That has two
> advantages:
>
>   1. It further decreases the size of "struct commit" for callers who do
>      not use save_commit_buffer.
>
>   2. It ensures that no new callers crop up who set "commit->buffer" but
>      to not save the size in the slab (you can see in the patch below
>      that I had to modify builtin/blame.c, which (ab)uses
>      commit->buffer).
>
> It would be more disruptive to existing callers, but I think the end
> result would be pretty clean. The API would be something like:
>
>   /* attach buffer to commit */
>   set_commit_buffer(struct commit *, void *buf, unsigned long size);
>
>   /* get buffer, either from slab cache or by calling read_sha1_file */
>   void *get_commit_buffer(struct commit *, unsigned long *size);
>
>   /* free() an allocated buffer from above, noop for cached buffer */
>   void unused_commit_buffer(struct commit *, void *buf);
>
>   /* drop saved commit buffer to free memory */
>   void free_commit_buffer(struct commit *);
>
> The "get" function would serve the existing callers in pretty.c, as well
> as the one I'm adding elsewhere in show_signature. And it should work as
> a drop-in read_sha1_file/free replacement for you here.

This solution has the kind of niceness to make everybody (certainly
including me) who has been involved in the code path should say "why
didn't I think of that!" ;-)