Re: [PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest

2017-10-09 Thread Junio C Hamano
Jeff King writes: > Actually, there are only two callers left these days. One of them leaks, > and the other immediately closes the zstream. So something like: > > diff --git a/sha1_file.c b/sha1_file.c > index 09ad64ce55..cea003d182 100644 > --- a/sha1_file.c > +++ b/sha1_file.c

Re: [PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest

2017-10-05 Thread Jeff King
On Fri, Oct 06, 2017 at 12:30:08AM -0400, Jeff King wrote: > On Fri, Oct 06, 2017 at 01:19:21PM +0900, Junio C Hamano wrote: > > > > But note that the leak in (2) is actually older than that. > > > The original unpack_sha1_file() directly returned the result > > > of unpack_sha1_rest() to its

Re: [PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest

2017-10-05 Thread Jeff King
On Fri, Oct 06, 2017 at 01:19:21PM +0900, Junio C Hamano wrote: > > But note that the leak in (2) is actually older than that. > > The original unpack_sha1_file() directly returned the result > > of unpack_sha1_rest() to its caller, when it should have > > been closing the zlib stream itself on

Re: [PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest

2017-10-05 Thread Junio C Hamano
Jeff King writes: > When a caller of sha1_object_info_extended() sets the > "contentp" field in object_info, we call unpack_sha1_rest() > but do not check whether it signaled an error. > > This causes two problems: > > 1. We pass back NULL to the caller via the contentp field, >

[PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest

2017-10-05 Thread Jeff King
When a caller of sha1_object_info_extended() sets the "contentp" field in object_info, we call unpack_sha1_rest() but do not check whether it signaled an error. This causes two problems: 1. We pass back NULL to the caller via the contentp field, but the function returns "0" for success. A