Re: Crash on MSYS2 with GIT_WORK_TREE
- Date: Wed, 8 Mar 2017 12:59:00 +0100 (CET)
- From: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
- Subject: Re: Crash on MSYS2 with GIT_WORK_TREE
On Tue, 7 Mar 2017, Brandon Williams wrote:
> On 03/08, Johannes Schindelin wrote:
> > [...] On *Linux*, this happens:
> > $ GIT_WORK_TREE=c:/invalid git rev-parse HEAD
> > Segmentation fault (core dumped)
> > The reason is this: when set_git_work_tree() was converted from using
> > xstrdup(real_path()) to real_pathdup(), we completely missed the fact
> > that the former passed die_on_error = 1 to strbuf_realpath(), while
> > the latter passed die_on_error = 0. As a consequence, work_tree can be
> > NULL now, and the current code does not expect set_git_work_tree() to
> > return successfully after setting work_tree to NULL.
> > I Cc:ed Brandon, the author of 4ac9006f832 (real_path: have callers
> > use real_pathdup and strbuf_realpath, 2016-12-12).
> > Brandon, I have a hunch that pretty much all of the
> > xstrdup(real_path()) -> real_pathdup() sites have a problem now. The
> > previous contract was that real_path() would die() if the passed path
> > is invalid. The new contract is that real_pathdup() returns NULL in
> > such a case. I believe that the following call sites are problematic
> > in particular:
> Welp, looks like I missed that when I made the conversion. You're
> right, the semantics of getting the real_path were changed which would
> cause a NULL to be returned instead of the program exiting with a call
> to die().
> After a cursory look at your patch, I think all of your changes look
> sane. I would have to take a closer look at the call sites to see if
> each caller would need to die or not. I'm assuming you took a quick
> glace to make your decision about each call site?
I did take a quick glance, but did you have a look at the time of day I
sent this patch? You do not want to trust my judgement after that.
Another thing: may I ask you to delete the quoted parts of the mail that
you are actually not responding to? Junio also often simply keeps the rest
of the mail quoted, and I always have to scroll all the way to the end
just to verify that nothing more has been said, which can be slightly
annoying when you are tired. I do plan to read your mails in the future,
so culling the quoted-yet-unanswered part would save me trouble.