Re: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Dec 16, 2014 at 2:29 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller <sbeller@google.com> wrote:
>> ---
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index e76e5d5..0803fd2 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
>>                 return;
>>         }
>>
>> +       if (use_atomic) {
>> +               transaction = ref_transaction_begin(&err);
>> +               if (!transaction) {
>> +                       error("%s", err.buf);
>> +                       strbuf_release(&err);
>> +                       for (cmd = commands; cmd; cmd = cmd->next)
>> +                               cmd->error_string = "transaction error";
>> +                       return;
>> +               }
>> +       }
>
> Not seen in this diff, but just below this point, the pre-receive hook
> is invoked. If it "declines", then execute_commands() returns without
> releasing the transaction which was begun here. Is that correct
> behavior?
>
> For robustness, it might also be sane to release the 'err' strbuf at
> this early return (though the current code does not strictly leak it).

To clarify: By "this early return", I mean the early return taken when
pre-receive hook declines.