Re: [PATCH v2] clone: Don't segfault on -b specifing a non-commit

From: Jeff King <peff@peff.net>
Date: Nov 6, 2019, 04:05 UTC
Message-ID: <20191106040523.GB4307@sigill.intra.peff.net>
In-Reply-To: <xmqqbltpyeel.fsf@gitster-ct.c.googlers.com>

On Wed, Nov 06, 2019 at 10:36:18AM +0900, Junio C Hamano wrote:

Show 22 quoted lines

> >   - we could probably simplify this by just inlining it into
> >     update_head(). Something like:
> >
> >       if (our)
> >               tip = &our->old_oid;
> >       else if (remote)
> >               tip = &remote->old_oid;
> >
> >       if (!tip) {
> > 	      /*
> > 	       * We have no local branch requested with "-b", and the
> > 	       * remote HEAD is unborn. There's nothing to update HEAD
> > 	       * to, but this state is not an error.
> > 	       */
> >               return 0;
> >       }
> 
> I somehow had an impression that Davide was protecting against the
> case where tip->old_oid is null_oid (cloning from an empty repo?);
> NULL return from lookup_commit_reference_gently(null_oid) would not
> deserve a warning from this codepath, and should work just like the
> way it has worked before these changes.

I'm not sure if we need to worry about that case. Right now it would just segfault, I think, which means it's not something that comes up regularly (though whether the _best_ thing might be to consider a non-error, I don't know without understanding why we'd see a null_oid in the first place).

If we did want to handle it, though, I think it would be easy with the setup I described; update_head() could check is_null_oid() itself.

-Peff