git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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:55 UTC
Message-ID
<20160517195541.GC11289@sigill.intra.peff.net>
In-Reply-To
<20160517195136.GB11289@sigill.intra.peff.net>
On Tue, May 17, 2016 at 03:51:37PM -0400, Jeff King wrote:
Show 31 quoted lines
> 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
Previous: Jeff KingNext: Noam Postavsky
Message 5 of 22 in “[BUG] A part of an edge from an octopus merge gets colored, even with --color=never”
  1. Noam PostavskyMay 15, 2016
  2. Johannes SixtMay 17, 2016
  3. Jeff KingMay 17, 2016
  4. Jeff KingMay 17, 2016
  5. Jeff KingMay 17, 2016
  6. Noam PostavskyMay 20, 2016
  7. Noam PostavskyJun 23, 2018
  8. Jeff KingJun 25, 2018
  9. Noam PostavskyJun 30, 2018
  10. Noam PostavskyAug 6, 2018
  11. Jeff KingAug 6, 2018
  12. Jeff KingAug 6, 2018
  13. Noam PostavskySep 2, 2018
  14. Jeff KingSep 8, 2018
  15. Noam PostavskySep 25, 2018
  16. Noam PostavskyOct 3, 2018
  17. Jeff KingOct 3, 2018
  18. Noam PostavskyOct 3, 2018
  19. Jeff KingOct 9, 2018
  20. Noam PostavskyOct 10, 2018
  21. Johannes SixtMay 17, 2016
  22. Jeff KingMay 17, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.