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:

> The files backend uses `ref_update::flags` for several internal flags.
> But those flags have no meaning to the packed backend. So when adding
> updates for the packed-refs transaction, only use flags that make
> sense to the packed backend.
>
> `REF_NODEREF` is part of the public interface, and it's logically what
> we want, so include it. In fact it is actually ignored by the packed
> backend (which doesn't support symbolic references), but that's its
> own business.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs/files-backend.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 2bd54e11ae..fadf1036d3 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2594,8 +2594,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
>  
>  			ref_transaction_add_update(
>  					packed_transaction, update->refname,
> -					update->flags & ~REF_HAVE_OLD,
> -					&update->new_oid, &update->old_oid,
> +					REF_HAVE_NEW | REF_NODEREF,
> +					&update->new_oid, NULL,

Hmph, so we earlier passed all flags except HAVE_OLD down, which
meant that update->flags that this transaction for packed backend
does not have to see are given to it nevertheless.  The new way the
parameter is prepared does nto depend on update->flags at all, so
that is about "don't leak flags".

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" and (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?



>  					NULL);
>  		}
>  	}