Re: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Dec 16, 2014, 20:30 UTC
- Message-ID
- <CAPig+cQu48Woi-TbbSe1=hsnh1EgGzV781gV_tZQwjVFoHy2Bg@mail.gmail.com>
- In-Reply-To
- <CAPig+cRkPZ8-MQyDs_S-3UXy2JUP6nLzjCE-pmpOa4Z4gyQKEg@mail.gmail.com>
On Tue, Dec 16, 2014 at 2:29 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 28 quoted lines
> 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.