Re: [PATCH v2 3/3] read_loose_refs(): treat NULL_SHA1 loose references as broken

From: Jeff King <peff@peff.net>
Date: Jun 3, 2015, 09:09 UTC
Message-ID: <20150603090903.GE32000@peff.net>
In-Reply-To: <556E1BBE.7000708@alum.mit.edu>

On Tue, Jun 02, 2015 at 11:10:22PM +0200, Michael Haggerty wrote:

Show 8 quoted lines

> >> +
> >> +                       if (!(flag & REF_ISBROKEN) && is_null_sha1(sha1)) {
> > 
> > Why do we do the extra check for !(flag & REF_ISBROKEN) here?
> 
> That was an attempt to avoid calling is_null_sha1() unnecessarily. I
> think I can make this go away and make the code clearer in general by
> restructuring the logic a little bit. I will do that in the next round.

If you get rid of the useless hashclr(), then this just becomes:

  if (!(flag & REF_ISBROKEN) && is_null_sha1(sha1))
	flag |= REF_ISBROKEN;

The reason for the initial check seems pretty obvious then (but it would also be OK without it; is_null_sha1 is not that expensive).

-Peff