From: Eric Sunshine <sunshine@sunshineco.com>
Date: Tue, 30 Dec 2014 09:10:08 GMT
Subject: Re: [PATCHv8 4/9] receive-pack.c: simplify execute_commands
Message-ID: <CAPig+cQEfGj8dH+FFOR4UYbNgovhFy_pP8szSpS+0MyNdOvJrw@mail.gmail.com>
In-Reply-To: <CAGZ79kYvv_r8TahfoE5g_5nBQhjsAC3tHvxUUBE4PtKLy8sGhQ@mail.gmail.com>

On Tue, Dec 30, 2014 at 3:42 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Dec 29, 2014 at 11:46 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Mon, Dec 29, 2014 at 9:36 PM, Stefan Beller <sbeller@google.com> wrote:
>>> No functional changes intended.
>>> +static void check_shallow_bugs(struct command *commands,
>>> +                              struct shallow_info *si)
>>> +{
>>> +       struct command *cmd;
>>> +       int checked_connectivity = 1;
>>> +       for (cmd = commands; cmd; cmd = cmd->next) {
>>> +               if (!should_process_cmd(cmd))
>>> +                       continue;
>>> +
>>> +               if (shallow_update && si->shallow_ref[cmd->index]) {
>>
>> Another issue: In the original code, 'si->shallow_ref[cmd->index]' is
>> only checked if cmd->error_string is NULL, but here you check it
>> unconditionally, despite the commit message claiming no functional
>> changes. Did you verify that such a behavioral change is benign? (This
>> is a genuine question.)
>
> The error != NULL check is done in if (!should_process_cmd(cmd))

Right. Thanks for setting me straight on that.