Re: [PATCH 1/7] files_transaction_prepare(): don't leak flags to packed transaction

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

>> That much I can understand.  But it is not explained why (1) we do
>> not pass old_oid anymore and (2) we do give HAVE_NEW.  
>> 
>> Presumably the justification for (1) is something like "because we
>> are not passing HAVE_OLD, we shouldn't have been passing old_oid at
>> all---it was a harmless bug because lack of HAVE_OLD made the callee
>> ignore old_oid"
>
> It's debatable whether the old code should even be called a bug. The
> callee is documented to ignore `old_oid` if `HAVE_OLD` is not set. But
> it was certainly misleading to pass unneeded information to the function.
>
>>                     (2) is "we need to pass HAVE_NEW, and we have
>> been always passing HAVE_NEW because update->flags at this point is
>> guaranteed to have it" or something like that?
>
> Correct. `REF_DELETING` is set by `lock_ref_for_update()` only if
> `update->flags & REF_HAVE_NEW` was set, and this code is only called if
> `REF_DELETING` is set.

It is is extra nice for you to give answers that are customized for
me like the above to my questions, but the questions came because
the log message did not answer them, so please make sure the next
person who did not read this exchange but reads only the resulting
commit does not have to ask the same question.

Thanks.