Hi Junio,
On Tue, 7 Mar 2017, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> 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