Subject: Re: [PATCH v2 26/27] struct ref_update: Add type field

From: Junio C Hamano <[email-masked]>
Date: Tue, 01 Apr 2014 13:03:27 -0700

Michael Haggerty <[email-masked]> writes:

> This is temporary space for ref_transaction_commit().
>
> Signed-off-by: Michael Haggerty <[email-masked]>
> ---

I was about to complain to "*Add* type" that does not say what it is
used for at all, with "Please do not add something for unknown purpose
only to utilise it in a later patch".

But that was before I noticed that these are already used and
realized that the change is about "moving what is recorded in the
type array, which is used to receive the existing reftype discovered
by calling resolve_ref_unsafe() in ref_transaction_commit() and not
used anywhere else, to a field of individual ref_update structure".

So it was somewhat of a "Huh?", but perhaps it is OK.

I wonder if ref-transaction-commit can shrink its parameter list by
accepting a single pointer to one ref_update?

>  refs.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index d1edd57..07f900a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3279,6 +3279,7 @@ struct ref_update {
>  	int flags; /* REF_NODEREF? */
>  	int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
>  	struct ref_lock *lock;
> +	int type;
>  	const char refname[FLEX_ARRAY];
>  };
>  
> @@ -3410,7 +3411,6 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>  {
>  	int ret = 0, delnum = 0, i;
>  	struct ref_update **updates;
> -	int *types;
>  	const char **delnames;
>  	int n = transaction->nr;
>  
> @@ -3419,7 +3419,6 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>  
>  	/* Allocate work space */
>  	updates = xmalloc(sizeof(*updates) * n);
> -	types = xmalloc(sizeof(*types) * n);
>  	delnames = xmalloc(sizeof(*delnames) * n);
>  
>  	/* Copy, sort, and reject duplicate refs */
> @@ -3437,7 +3436,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>  					       (update->have_old ?
>  						update->old_sha1 : NULL),
>  					       update->flags,
> -					       &types[i], onerr);
> +					       &update->type, onerr);
>  		if (!update->lock) {
>  			ret = 1;
>  			goto cleanup;
> @@ -3465,7 +3464,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>  
>  		if (update->lock) {
>  			delnames[delnum++] = update->lock->ref_name;
> -			ret |= delete_ref_loose(update->lock, types[i]);
> +			ret |= delete_ref_loose(update->lock, update->type);
>  		}
>  	}
>  
> @@ -3479,7 +3478,6 @@ cleanup:
>  		if (updates[i]->lock)
>  			unlock_ref(updates[i]->lock);
>  	free(updates);
> -	free(types);
>  	free(delnames);
>  	ref_transaction_free(transaction);
>  	return ret;