On Tue, May 17, 2016 at 03:51:37PM -0400, Jeff King wrote:
> 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.
And unsurprisingly, reverting 339c17bc7690b5436ac61c996cede3d52c85b50d
seems to fix this (author cc'd). It's the extra "commit_index" addition
that causes the problem. But I'm still not sure what the correct
solution is.
-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