Re: [BUG] A part of an edge from an octopus merge gets colored, even with --color=never

From: Jeff King <peff@peff.net>
Date: May 17, 2016, 19:51 UTC
Message-ID: <20160517195136.GB11289@sigill.intra.peff.net>
In-Reply-To: <20160517194533.GA11289@sigill.intra.peff.net>

On Tue, May 17, 2016 at 03:45:34PM -0400, Jeff King wrote:

> Note that we set up f90, fa0, and fb0, but then pass fc0 into
> strbuf_write_column (and it has bogus color values). It looks like we're
> reading one past the end of our array, but I haven't figured out where
> or why.

Looking at the valgrind output reveals that. Here's an assert() that catches it reliably for me:

diff --git a/graph.c b/graph.c
index 1350bdd..964bbd1 100644
--- a/graph.c
+++ b/graph.c
@@ -794,9 +794,11 @@ static int graph_draw_octopus_merge(struct git_graph *graph,
 		((graph->num_parents - dashless_commits) * 2) - 1;
 	for (i = 0; i < num_dashes; i++) {
 		col_num = (i / 2) + dashless_commits + graph->commit_index;
+		assert(col_num < graph->num_new_columns);
 		strbuf_write_column(sb, &graph->new_columns[col_num], '-');
 	}
 	col_num = (i / 2) + dashless_commits + graph->commit_index;
+	assert(col_num < graph->num_new_columns);
 	strbuf_write_column(sb, &graph->new_columns[col_num], '.');
 	return num_dashes + 1;
 }

(It's actually the first one which triggers). I'm not familiar enough
with the code to know whether the col_num computation is bogus, or
whether we needed to earlier increase the size of the "new_columns"
field.

-Peff