Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

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

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

> It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING
> without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING
> caller to pass REF_NODEREF too.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> This also makes later patches a bit clearer.

I wonder if it is more future-proof to solve this by doing

    -#define REF_ISPRUNING	0x04
    +#define REF_ISPRUNING	(0x04 | REF_NODEREF)

instead.  It makes the intention clear that pruning is always about
the single level (i.e. no-deref).


>  refs.c               | 3 +++
>  refs/files-backend.c | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index ba14105..5dc2473 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -790,6 +790,9 @@ 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 ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
> +		die("BUG: REF_ISPRUNING set without REF_NODEREF");
> +
>  	if (new_sha1 && !is_null_sha1(new_sha1) &&
>  	    check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
>  		strbuf_addf(err, "refusing to update ref with bad name '%s'",
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 9faf17c..8fcbd7d 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2116,7 +2116,7 @@ static void prune_ref(struct ref_to_prune *r)
>  	transaction = ref_transaction_begin(&err);
>  	if (!transaction ||
>  	    ref_transaction_delete(transaction, r->name, r->sha1,
> -				   REF_ISPRUNING, NULL, &err) ||
> +				   REF_ISPRUNING | REF_NODEREF, NULL, &err) ||
>  	    ref_transaction_commit(transaction, &err)) {
>  		ref_transaction_free(transaction);
>  		error("%s", err.buf);