Re: [PATCH 1/7] files_transaction_prepare(): don't leak flags to packed transaction
From: Junio C Hamano <hidden>
Date: 2017-10-30 04:44:13
Michael Haggerty [off-list ref] writes:
quoted hunk
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 <redacted> --- 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); } }