[PATCH 0/3] lazily load commit->buffer

[Jeff King <peff@peff.net>]

On Fri, Jan 25, 2013 at 07:36:18AM -0800, Junio C Hamano wrote:

> Jonathon Mah <jmah@me.com> writes:
> 
> > Just to note, the proposals so far don't prevent a "smart-ass"
> > function from freeing the buffer when it's called underneath the
> > use/release scope, as in:
> >
> > with_commit_buffer(commit); {
> > 	fn1_needing_buffer(commit);
> > 	walk_rev_tree_or_something();
> > 	fn2_needing_buffer(commit);
> > } done_with_commit_buffer(commit);
> 
> I think the goal of everybody discussing these ideas is to make sure
> that all code follows the simple ownership policy proposed at the
> beginning of this subthread: commit->buffer belongs to the revision
> traversal machinery, and other users could borrow it when available.

Yeah, agreed. I started to fix this up with a use/unuse pattern and
realized something: all of the call sites are calling logmsg_reencode
anyway, because that is the next logical step in doing anything with the
buffer that is not just parsing out the parent/timestamp/tree info. And
since that function already might allocate (for the re-encoded copy),
callers have to handle the maybe-borrowed-maybe-free situation already.

So I came up with this patch series, which I think should fix the
problem, and actually makes the call-sites easier to read, rather than
harder.

  [1/3]: commit: drop useless xstrdup of commit message
  [2/3]: logmsg_reencode: never return NULL
  [3/3]: logmsg_reencode: lazily load missing commit buffers

Here's the diffstat:

 builtin/blame.c                  | 22 ++-------
 builtin/commit.c                 | 14 +-----
 commit.h                         |  1 +
 pretty.c                         | 93 ++++++++++++++++++++++++++---------
 t/t4042-diff-textconv-caching.sh |  8 +++
 5 files changed, 85 insertions(+), 53 deletions(-)

Not too bad, and 27 of the lines added in pretty.c are new comments
explaining the flow of logmsg_reencode. So even if this doesn't get
every case, I think it's a nice cleanup.

-Peff