Re: [PATCH v2 04/12] ref_transaction_update(): remove "have_old" parameter

On 02/12/2015 06:32 PM, Junio C Hamano wrote:

On Thu, Feb 12, 2015 at 3:12 AM, Michael Haggerty [off-list ref] wrote:

quoted

Instead, verify the reference's old value if and only if old_sha1 is
non-NULL.

...

@@ -3664,9 +3664,6 @@ int ref_transaction_update(struct ref_transaction *transaction,
        if (transaction->state != REF_TRANSACTION_OPEN)
                die("BUG: update called for transaction that is not open");

-       if (have_old && !old_sha1)
-               die("BUG: have_old is true but old_sha1 is NULL");
-

In the old world, old_sha1 here used to be one of
 (1) NULL, (2) null_sha1[], or (3) a real object name.
What is the rule in the new world?

The new world is explained in the docstring in refs.h, which was updated
in this same commit:

    If old_sha1 is non-NULL, then it the value
    that the reference should have had before the update, or null_sha1
    if it must not have existed beforehand.

The docstring is further revised later in the patch series to

    old_sha1 is the value that the reference must have
    before the update, or null_sha1 if it must not have existed
    beforehand. The old value is checked after the lock is taken to
    prevent races. If the old value doesn't agree with old_sha1, the
    whole transaction fails. If old_sha1 is NULL, then the previous
    value is not checked.

The new rule is extended to ref_transaction_delete() in the subsequent
commit. I like the new semantics because it removes redundancy in the
interpretation of parameters.

Is that explanation adequate?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu