git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Davide Berardi <berardi.dav@gmail.com>,
	git@vger.kernel.org, peff@peff.net
Subject: Re: [PATCH] Segmentation Fault on non-commit --branch clone
Date: Sat, 02 Nov 2019 18:18:44 +0900	[thread overview]
Message-ID: <xmqqtv7m1vrf.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1911012000160.46@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Fri, 1 Nov 2019 20:08:10 +0100 (CET)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Davide,
>
> I wonder whether you might want to reword the Subject: such that it
> reflects what the patch does instead of what the problem is that
> motivated you to work on the patch (the patch does not cause the
> segmentation fault, after all, but it fixes it).

Good point.

    Subject: clone: do not segfault on "clone -b B" where B is a non-commit

    The code in "git clone -b B" to decide what branch to check out
    assumed that B points at a commit object without checking,
    leading to dereferencing a NULL pointer and causing a segfault.

    Just aborting the operation when it happens is not a very
    attractive option because we would be left with a directory
    without .git/HEAD that cannot be used as a valid repository the
    user can attempt to recover from the situation by checking out
    something.

    Fall back to use the 'master' branch, which is what we use when
    the command without the "-b" option cannot figure out what
    branch the remote side points with its HEAD.

or something like that, perhaps?

I am not sure if the existing code is careful enough setting up the
resulting local 'master' branch, or needs more changes associated
with this patch, though.  For example, does it want to be set to
integrate with the 'master' branch on the remote side by setting
"branch.master.remote" and "branch.master.merge" configuration, or
do we want to turn them off?  I _think_ the answer is that we want
to behave as if the user said "-b master" instead of "-b B" (with B
that does not point at a commit), but I am not sure.  Also, don't we
try to be a bit noisier when the fallback fails?  For example, if
the user said "clone -b master" and the 'master' points at an object
that is not a commit, falling back and writing refs/heads/master in
the HEAD would leave us in the same position as we did not have any
fallback.

I skimmed your review and I think I agree with most (if not all) of
them.  Thanks.

  parent reply	other threads:[~2019-11-02  9:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01  0:24 Davide Berardi
2019-11-01 19:08 ` Johannes Schindelin
2019-11-01 19:35   ` Jeff King
2019-11-02 10:16     ` Junio C Hamano
2019-11-03 18:16       ` Davide Berardi
2019-11-04  3:55         ` Junio C Hamano
2019-11-02  9:18   ` Junio C Hamano [this message]
2019-11-01 19:43 ` Jeff King
2019-11-02 10:07 ` Junio C Hamano
2019-11-03 18:07 ` [PATCH v2] clone: Don't segfault on -b specifing a non-commit Davide Berardi
2019-11-05  4:37   ` Jeff King
2019-11-06  1:36     ` Junio C Hamano
2019-11-06  4:05       ` Jeff King
2019-11-06  9:53         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtv7m1vrf.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=berardi.dav@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --subject='Re: [PATCH] Segmentation Fault on non-commit --branch clone' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).