From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 29 Apr 2014 22:10:05 GMT
Subject: Re: [PATCH v4 15/27] fast-import.c: change update_branch to use ref transactions
Message-ID: <xmqqmwf3iy82.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1398725682-30782-16-git-send-email-sahlberg@google.com>

Ronnie Sahlberg <sahlberg@google.com> writes:

> +	transaction = ref_transaction_begin();
> +	if ((!transaction ||
> +	    ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
> +				   0, 1)) ||
> +	    (ref_transaction_commit(transaction, msg, &err) &&
> +	     !(transaction = NULL))) {
> +		ref_transaction_rollback(transaction);
> +		return error("Unable to update branch %s: %s", b->name,
> +			     strbuf_detach(&err, NULL));
> +	}

The assigning of NULL to transaction and always returning true looks
very suspicious.

Also, if we couldn't get transaction what happens?  I think you
wanted to avoid passing NULL to rollback but at the same time I
suspect you wanted to fall into that "return error()" part, but
apparently you are failing to have that cake and eat it too, no?