From: Ronnie Sahlberg <sahlberg@google.com>
Date: Wed, 28 May 2014 21:14:35 GMT
Subject: Re: [PATCH v11 23/41] refs.c: change update_ref to use a transaction
Message-ID: <CAL=YDWkCN1ZW48YM3pwVoRLu4-TPwa4cr1m420H+LLx=DBvM=g@mail.gmail.com>
In-Reply-To: <20140528193139.GW12314@google.com>

Done.
Thanks.

On Wed, May 28, 2014 at 12:31 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ronnie Sahlberg wrote:
>
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3474,11 +3474,28 @@ int update_ref(const char *action, const char *refname,
>>              const unsigned char *sha1, const unsigned char *oldval,
>>              int flags, enum action_on_err onerr)
>>  {
>> -     struct ref_lock *lock;
>> -     lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
>> -     if (!lock)
>> +     struct ref_transaction *t;
>> +     struct strbuf err = STRBUF_INIT;
>> +
>> +     t = ref_transaction_begin(&err);
>> +     if ((!t ||
>> +         ref_transaction_update(t, refname, sha1, oldval, flags,
>> +                                !!oldval, &err)) ||
>
> (style) Extra parens.
>
>> +         (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
>
> No need for this assignment-in-if.
>
> With the following squashed in,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> diff --git i/refs.c w/refs.c
> index 568b358..fb462a3 100644
> --- i/refs.c
> +++ w/refs.c
> @@ -3474,10 +3474,10 @@ int update_ref(const char *action, const char *refname,
>         struct strbuf err = STRBUF_INIT;
>
>         t = ref_transaction_begin(&err);
> -       if ((!t ||
> +       if (!t ||
>             ref_transaction_update(t, refname, sha1, oldval, flags,
> -                                  !!oldval, &err)) ||
> -           (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
> +                                  !!oldval, &err) ||
> +           ref_transaction_commit(t, action, &err)) {
>                 const char *str = "update_ref failed for ref '%s': %s";
>
>                 ref_transaction_free(t);