On Fri, Jul 22, 2011 at 10:33:31AM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > From my quick look, I came up with the fix below. It removes the leak
> > and doesn't trigger any memory errors according to valgrind. So it
> > _must_ be right. :)
> >
> > -Peff
> >
> > ---
> > diff --git a/streaming.c b/streaming.c
> > index 565f000..f3acc5d 100644
> > --- a/streaming.c
> > +++ b/streaming.c
> > @@ -93,7 +93,9 @@ struct git_istream {
> >
> > int close_istream(struct git_istream *st)
> > {
> > - return st->vtbl->close(st);
> > + int r = st->vtbl->close(st);
> > + free(st);
> > + return r;
> > }
>
> I do not think of a reason any future callers would want to close the
> stream first and then inspect some attribute of the stream afterwards (if
> this were an output stream, there might be things like output stats that
> may want such an interface, but even in such a case, the caller should say
> "flush" first to drain whatever is pending, grab stats and then close), so
> freeing the resource at close time seems sensible to me.
Yeah. My impression was that these istreams would operate much like
"FILE *" streams. And closing those frees all resources, and you can
throw away the pointer afterwards.
> Both the external caller in entry.c and the internal caller (a filtered
> istream reads from its underlying istream, and when it is getting closed,
> closes the underlying istream) were leaking the istream exactly the same
> way, so this fix should plug both of them.
>
> Thanks.
Did you want me to write a commit message? I think you might do a better
job of writing a coherent one for this particular patch.
-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