On Thu, Jan 24, 2013 at 07:59:58PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > 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.
>
> I've been toying with an idea along this line.
>
> commit.h | 16 ++++++++++++++++
> builtin/blame.c | 27 ++++++++-------------------
> commit.c | 20 ++++++++++++++++++++
> 3 files changed, 44 insertions(+), 19 deletions(-)
I think we are on the same page as far as what needs to happen at the
call sites.
My suggested implementation had a separate buffer, but you are right
that we may need to actually set "commit->buffer" because sub-functions
expect to find it there (the alternative might be cleaning up the
sub-function interfaces). I haven't looked at the call-sites yet.
This:
> +extern int ensure_commit_buffer(struct commit *);
> +extern void discard_commit_buffer(struct commit *);
> +
> +#define with_commit_buffer(commit) \
> + do { \
> + int had_buffer_ = !!commit->buffer; \
> + if (!had_buffer_) \
> + ensure_commit_buffer(commit); \
> + do
> +
> +#define done_with_commit_buffer(commit) \
> + while (0); \
> + if (!had_buffer_) \
> + discard_commit_buffer(commit); \
> + } while (0)
is pretty nasty, though. I know it gets the job done, but in my
experience, macros which do not behave syntactically like functions are
usually a good sign that you are doing something gross and
unmaintainable.
I dunno.
-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