Junio C Hamano gits...@pobox.com writes:
John Keeping j...@keeping.me.uk writes:
We are guaranteed that 'nst' is non-null because it is allocated with
xmalloc(), and in fact we rely on this three lines later by
unconditionally dereferencing it.
The intent of the original code is for attach_stream_filter() to
detect an error condition and return NULL, in which case it closes
the istream it allocated and signal error to the caller, I think,
and falling thru to use st-anything and return st when that happens
is *not* a guarantee that a-s-f will not detect an error ever, but
rather is a bug in the error codepath.
In other words, something like this instead.
-- 8 --
Subject: [PATCH] open_istream(): do not dereference NULL in the error case
When stream-filter cannot be attached, it is expected to return NULL,
and we should close the stream we opened and signal an error by
returning NULL ourselves from this function.
However, we attempted to dereference that NULL pointer between the
point we detected the error and returned from the function.
Brought-to-attention-by: John Keeping j...@keeping.me.uk
Signed-off-by: Junio C Hamano gits...@pobox.com
---
streaming.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/streaming.c b/streaming.c
index d7c9f32..2ff036a 100644
--- a/streaming.c
+++ b/streaming.c
@@ -152,8 +152,10 @@ struct git_istream *open_istream(const unsigned char *sha1,
if (filter) {
/* Add !is_null_stream_filter(filter) for performance */
struct git_istream *nst = attach_stream_filter(st, filter);
- if (!nst)
+ if (!nst) {
close_istream(st);
+ return NULL;
+ }
st = nst;
}
--
1.9.0-279-gdc9e3eb
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
