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.
next prev 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 [PATCH] Segmentation Fault on non-commit --branch clone 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
List information: http://vger.kernel.org/majordomo-info.html
* 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 \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).