Subject: Re: [PATCH v11 14/41] refs.c: update ref_transaction_delete to check for error and return status

From: Jonathan Nieder <[email-masked]>
Date: Wed, 28 May 2014 11:42:34 -0700

Ronnie Sahlberg wrote:

> Signed-off-by: Ronnie Sahlberg <[email-masked]>
> ---
>  builtin/update-ref.c |  5 +++--
>  refs.c               | 16 +++++++++++-----
>  refs.h               | 12 ++++++++----
>  3 files changed, 22 insertions(+), 11 deletions(-)

Reviewed-by: Jonathan Nieder <[email-masked]>

[...]
> +++ b/refs.c
> @@ -3417,19 +3417,25 @@ int ref_transaction_create(struct ref_transaction *transaction,
>  	return 0;
>  }
>  
> -void ref_transaction_delete(struct ref_transaction *transaction,
> -			    const char *refname,
> -			    const unsigned char *old_sha1,
> -			    int flags, int have_old)
> +int ref_transaction_delete(struct ref_transaction *transaction,
> +			   const char *refname,
> +			   const unsigned char *old_sha1,
> +			   int flags, int have_old,
> +			   struct strbuf *err)
>  {
> -	struct ref_update *update = add_update(transaction, refname);
> +	struct ref_update *update;
>  
> +	if (have_old && !old_sha1)
> +		die("BUG: have_old is true but old_sha1 is NULL");
> +
> +	update = add_update(transaction, refname);
>  	update->flags = flags;
>  	update->have_old = have_old;
>  	if (have_old) {
>  		assert(!is_null_sha1(old_sha1));

Could combine this into the 'if (have_old &&' check.

Thanks,
Jonathan