Re: [PATCH v2 24/27] ref_transaction_commit(): Introduce temporary variables
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 1, 2014, 19:26 UTC
- Message-ID
- <xmqqy4zozw9q.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1395683820-17304-25-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Use temporary variables in the for-loop blocks to simplify expressions > in the rest of the loop.
Shouldn't the summary of the change "simplify expressions"? Use of temporary variables is a means to the end. If you have enough room to say "achieve X by doing Y", please do so; otherwise "achieve X" is more important part than "do Y".
Other than that, this looks good.
Show 50 quoted lines
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 2b80f6d..d51566c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3432,10 +3432,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>
> /* Acquire all locks while verifying old values */
> for (i = 0; i < n; i++) {
> - locks[i] = update_ref_lock(updates[i]->refname,
> - (updates[i]->have_old ?
> - updates[i]->old_sha1 : NULL),
> - updates[i]->flags,
> + struct ref_update *update = updates[i];
> +
> + locks[i] = update_ref_lock(update->refname,
> + (update->have_old ?
> + update->old_sha1 : NULL),
> + update->flags,
> &types[i], onerr);
> if (!locks[i]) {
> ret = 1;
> @@ -3444,16 +3446,19 @@ int ref_transaction_commit(struct ref_transaction *transaction,
> }
>
> /* Perform updates first so live commits remain referenced */
> - for (i = 0; i < n; i++)
> - if (!is_null_sha1(updates[i]->new_sha1)) {
> + for (i = 0; i < n; i++) {
> + struct ref_update *update = updates[i];
> +
> + if (!is_null_sha1(update->new_sha1)) {
> ret = update_ref_write(msg,
> - updates[i]->refname,
> - updates[i]->new_sha1,
> + update->refname,
> + update->new_sha1,
> locks[i], onerr);
> locks[i] = NULL; /* freed by update_ref_write */
> if (ret)
> goto cleanup;
> }
> + }
>
> /* Perform deletes now that updates are safely completed */
> for (i = 0; i < n; i++)