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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> This is temporary space for ref_transaction_commit().
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---

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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]