Re: [PATCHv8 4/9] receive-pack.c: simplify execute_commands

[Stefan Beller <sbeller@google.com>]

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.
>>
>> This commit shortens execute_commands by moving some parts of the code
>> to extra functions.
>>
>> Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index 4e8eaf7..06eb287 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -1043,11 +1043,40 @@ static void reject_updates_to_hidden(struct command *commands)
>>         }
>>  }
>>
>> +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))

>
>> +                       error("BUG: connectivity check has not been run on ref %s",
>> +                             cmd->ref_name);
>> +                       checked_connectivity = 0;
>> +               }
>> +       }
>> +       if (shallow_update && !checked_connectivity)
>> +               error("BUG: run 'git fsck' for safety.\n"
>> +                     "If there are errors, try to remove "
>> +                     "the reported refs above");
>> +}
>> +
>>  static void execute_commands(struct command *commands,
>>                              const char *unpacker_error,
>>                              struct shallow_info *si)
>>  {
>> -       int checked_connectivity;
>>         struct command *cmd;
>>         unsigned char sha1[20];
>>         struct iterate_data data;
>> @@ -1078,27 +1107,13 @@ static void execute_commands(struct command *commands,
>>         free(head_name_to_free);
>>         head_name = head_name_to_free = resolve_refdup("HEAD", 0, sha1, NULL);
>>
>> -       checked_connectivity = 1;
>>         for (cmd = commands; cmd; cmd = cmd->next) {
>> -               if (cmd->error_string)
>> -                       continue;
>> -
>> -               if (cmd->skip_update)
>> +               if (!should_process_cmd(cmd))
>>                         continue;
>>
>>                 cmd->error_string = update(cmd, si);
>> -               if (shallow_update && !cmd->error_string &&
>> -                   si->shallow_ref[cmd->index]) {
>> -                       error("BUG: connectivity check has not been run on ref %s",
>> -                             cmd->ref_name);
>> -                       checked_connectivity = 0;
>> -               }
>>         }
>> -
>> -       if (shallow_update && !checked_connectivity)
>> -               error("BUG: run 'git fsck' for safety.\n"
>> -                     "If there are errors, try to remove "
>> -                     "the reported refs above");
>> +       check_shallow_bugs(commands, si);
>>  }
>>
>>  static struct command **queue_command(struct command **tail,
>> --
>> 2.2.1.62.g3f15098