Re: [PATCHv8 4/9] receive-pack.c: simplify execute_commands
On Mon, Dec 29, 2014 at 11:46 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 33 quoted lines
> 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))
Show 52 quoted lines
>
>> + 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.g3f15098Stefan Beller <sbeller@google.com>