Re: [PATCH] get_oid: handle NULL repo->index

[Duy Nguyen <pclouds@gmail.com>]

On Wed, May 15, 2019 at 12:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > Also from my earlier message, if you missed it:
> >
> >   I also wondered if we should simply allocate an empty index whenever
> >   we have a non-toplevel "struct repository", which might be less
> >   surprising to other callers. I don't have a strong opinion either way.
> >   I did grep around for other callers which might have similar problems,
> >   but couldn't find any.
>
> That is an approach to make it harder to make mistakes by accepting
> possibly a small wasted resource; but at that point, I think calling
> repo_read_index() unconditionally from here and similar places would
> be a simpler fix in the same spirit.

For repo_read_index() case, maybe. But we have a lot of
"r(epo)->index->something". All or most of these references
traditionally are the_index.something, which is always safe to
dereference. Submodule repos with the optionally NULL repo->index
break this assumption.

So either we audit all the code and make sure "repo->index" cannot be
NULL because repo_read_index() has been called before (and may have to
repeat auditing), or we just stick to the old assumption and make sure
repo->index is not NULL from the beginning. This makes me think the
small extra resource is worth it. Much less time will be spent on
similar issues now and in the future.

PS. Sorry Jeff for the noise. I should have waited until I come home
and can read your mail more carefully.
-- 
Duy