Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

From: Stefan Beller <sbeller@google.com>
Date: Jul 22, 2016, 20:00 UTC
Message-ID: <CAGZ79kaoxo9qM-r9nNzG8GV4aCfun4RaK8jpsdSgbyix1bJBFg@mail.gmail.com>
In-Reply-To: <xmqq60rxa1sl.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 22, 2016 at 12:52 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> The approach to tests is different though. I like yours better than mine,
>> as it doesn't add more tests, but strengthens existing tests.
>
> So... are you retracting
> http://thread.gmane.org/gmane.comp.version-control.git/299995 and
> instead giving an Ack to these two?
>

I like this series better
* for the approach
* for the tests
* for the commit message

So I do think this should be applied instead of what I sent.

I am tempted to send a squash proposal like: (broken whitespaces)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6f6d67a..77be97e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -796,6 +796,8 @@ static int update_clone_task_finished(int result,
  return 0;
  } else {
  idx -= suc->list.nr;
+ if (idx >= suc->failed_clones_nr)
+ die("BUG: idx too large???");
  ce  = suc->failed_clones[idx];
  strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
     ce->name);