From: David Turner <dturner@twopensource.com>
Date: Wed, 27 Apr 2016 20:23:57 GMT
Subject: Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning
Message-ID: <1461788637.11504.3.camel@twopensource.com>
In-Reply-To: <xmqq60v2anyo.fsf@gitster.mtv.corp.google.com>

On Wed, 2016-04-27 at 11:47 -0700, Junio C Hamano wrote:
> 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).

I think the approach in Michael's patch might be clearer than yours,
since someone reading the code doesn't have to look at the definition
of REF_ISPRUNING to understand what is going on.