Re: regression in multi-threaded git-pack-index

[Jeff King <peff@peff.net>]

On Tue, Mar 19, 2013 at 05:59:43AM -0400, Jeff King wrote:

> > Yes, that has been my experience with valgrind false positives, too. But
> > if this is a real problem, it may be different from the OP's issue. It
> > seems to trigger for me in v1.7.10, before Duy's threading patches. It
> > does not seem to be in v1.7.5. I'm bisecting now.
> 
> Hmph. It bisects to Junio's d1a0ed1 (index-pack: show histogram when
> emulating "verify-pack -v", 2011-06-03), which introduces those lines.
> The deepest_delta variable is static, so by definition it is always
> initialized to something. So I guess some objects may not have
> delta_depth set. Still looking.

I'm doubly confused now. The commit in question introduces this:

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index aa3c9c6..ed4c3bb 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -70,6 +70,7 @@ static off_t consumed_bytes;
 static unsigned char input_buffer[4096];
 static unsigned int input_offset, input_len;
 static off_t consumed_bytes;
+static unsigned deepest_delta;
 static git_SHA_CTX input_ctx;
 static uint32_t input_crc32;
 static int input_fd, output_fd, pack_fd;
@@ -538,6 +539,8 @@ static void resolve_delta(struct object_entry *delta_obj,
 
 	delta_obj->real_type = base->obj->real_type;
 	delta_obj->delta_depth = base->obj->delta_depth + 1;
+	if (deepest_delta < delta_obj->delta_depth)
+		deepest_delta = delta_obj->delta_depth;
 	delta_obj->base_object_no = base->obj - objects;
 	delta_data = get_data_from_pack(delta_obj);
 	base_data = get_base_data(base);

and valgrind reports an uninitialized value in the conditional. But we
can see that deepest_delta is static, and therefore always has some
value. And delta_obj->delta_depth is set in the line above. So both
should have some known value, unless they are computed from unknown
values. In that case, shouldn't valgrind have previously noticed when we
accessed those unknown values?

-Peff