On 03/02/2015 10:44 PM, Junio C Hamano wrote:
> Michael Haggerty <mhag...@alum.mit.edu> writes:
> 
>> Instead, compute the value when it is needed.
> 
>> @@ -2318,8 +2317,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
>> *refname,
>>      lock->ref_name = xstrdup(refname);
>>      lock->orig_ref_name = xstrdup(orig_refname);
>>      ref_file = git_path("%s", refname);
>> -    if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
>> -            lock->force_write = 1;
>>  
>>   retry:
>>      switch (safe_create_leading_directories(ref_file)) {
>> @@ -3787,8 +3784,13 @@ int ref_transaction_commit(struct ref_transaction 
>> *transaction,
>>              struct ref_update *update = updates[i];
>>  
>>              if (!is_null_sha1(update->new_sha1)) {
>> -                    if (!update->lock->force_write &&
>> -                        !hashcmp(update->lock->old_sha1, update->new_sha1)) 
>> {
>> +                    if (!((update->type & REF_ISSYMREF)
>> +                          && (update->flags & REF_NODEREF))
>> +                        && !hashcmp(update->lock->old_sha1, 
>> update->new_sha1)) {
>> +                            /*
>> +                             * The reference already has the desired
>> +                             * value, so we don't need to write it.
>> +                             */
>>                              unlock_ref(update->lock);
>>                              update->lock = NULL;
>>                      } else if (write_ref_sha1(update->lock, 
>> update->new_sha1,
> 
> The code before and after the change are equivalent.
> 
> It shouldn't be the case, but somehow I find the original slightly
> easier to understand. [...]

I had the same feeling; thanks for confirming it. How about I introduce
a temporary variable `overwriting_symref` as an aid to the reader? I
think this makes it pretty clear:

>               if (!is_null_sha1(update->new_sha1)) {
> -                     if (!update->lock->force_write &&
> -                         !hashcmp(update->lock->old_sha1, update->new_sha1)) 
> {
> +                     int overwriting_symref = ((update->type & REF_ISSYMREF) 
> &&
> +                                               (update->flags & 
> REF_NODEREF));
> +
> +                     if (!overwriting_symref
> +                         && !hashcmp(update->lock->old_sha1, 
> update->new_sha1)) {
> +                             /*
> +                              * The reference already has the desired
> +                              * value, so we don't need to write it.
> +                              */
>                               unlock_ref(update->lock);
>                               update->lock = NULL;
>                       } else if (write_ref_sha1(update->lock, 
> update->new_sha1,

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to