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

From: Jeff King <[email-masked]>
Date: Wed, 3 Jun 2015 05:09:03 -0400

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

> >> +
> >> +                       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