From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff King Subject: Re: [BUG] A part of an edge from an octopus merge gets colored, even with --color=never Date: Tue, 17 May 2016 15:55:41 -0400 Message-ID: <20160517195541.GC11289@sigill.intra.peff.net> References: <573B6BF5.1090004@kdbg.org> <20160517194533.GA11289@sigill.intra.peff.net> <20160517195136.GB11289@sigill.intra.peff.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Hemmo Nieminen , Noam Postavsky , git@vger.kernel.org To: Johannes Sixt X-From: git-owner@vger.kernel.org Tue May 17 21:55:50 2016 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1b2l5p-0000b8-7I for gcvg-git-2@plane.gmane.org; Tue, 17 May 2016 21:55:49 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751120AbcEQTzp (ORCPT ); Tue, 17 May 2016 15:55:45 -0400 Received: from cloud.peff.net ([50.56.180.127]:40938 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750758AbcEQTzo (ORCPT ); Tue, 17 May 2016 15:55:44 -0400 Received: (qmail 11345 invoked by uid 102); 17 May 2016 19:55:44 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.84) with SMTP; Tue, 17 May 2016 15:55:44 -0400 Received: (qmail 17442 invoked by uid 107); 17 May 2016 19:55:45 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.84) with SMTP; Tue, 17 May 2016 15:55:45 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Tue, 17 May 2016 15:55:41 -0400 Content-Disposition: inline In-Reply-To: <20160517195136.GB11289@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: 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