Re: Crash on MSYS2 with GIT_WORK_TREE

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 7 Mar 2017, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > OK, so it appears that we'd better audit all the callsites of
> > real_pathdup() and see if anybody _assumes_ that the return values are
> > not NULL.  They all need fixing.

Indeed.

> I just looked at 4ac9006f ("real_path: have callers use real_pathdup and
> strbuf_realpath", 2016-12-12) and it seems all hunks that replaces
> xstrdup(real_path(...)) with real_pathdup(...) in the commit share the
> same issue.  

Right, I tried to convey that information in my email to which you
replied.

> The one in canonicalize_ceiling_entry() looks OK, though.

Yes, it immediately tests whether NULL was returned.

> ec9629b3 ("submodule absorbing: fix worktree/gitdir pointers
> recursively for non-moves", 2017-01-25) introduces a new use of
> real_pathdup() and the result is immediately used to call
> connect_work_tree_and_git_dir() without checking its NULL-ness, but
> the argument to new_git_dir is something that came from git_path()
> that was successfully passed to safe_create_leading_directories(),
> so this one should be OK.
> 
> 1c16df23 ("Merge branch 'bw/realpath-wo-chdir'", 2017-01-18) turns a
> few xstrdup(real_path(...)) in dir.c without thinking.  I think that
> evil merge probably should be reverted.

Rather than a heavy-handed reversal, I would really prefer to perform a
diligent audit of all real_pathdup() callers and adjust them
appropriately.

Turns out that the canonicalize_ceiling_entry() caller is *the only one*
handling NULL correctly. All other callers need to be changed.

Will send something out in a moment.

Ciao,
Johannes