On Fri, Jan 25, 2013 at 07:36:18AM -0800, Junio C Hamano wrote:
> Jonathon Mah <jmah@xxxxxx> 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
--
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