Re: [PATCH v4] cache-tree.c: remove implicit dependency on the_repository

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2021, 13:23 UTC
Message-ID: <xmqqmtu8vrk6.fsf@gitster.g>
In-Reply-To: <0df415b7-73b7-6e72-1885-3ca606acf8fd@gmail.com>

Chinmoy Chakraborty <chinmoy12c@gmail.com> writes:

> Is it safe to make this assumption?

It was the question I asked, and I didn't see a reason to believe it is safe.

> I mean to be completely sure of this, one would have to track back
> to all the callers.

Yes. If we audit the callers to make sure istate->repo always points at the right repository, add missing assignment to istate->repo as necessary, and add

	if (!istate->repo)
		BUG("caller of cache_tree_udpate() did not fill istate->repo");

then that would be an improvement.  But...

Show 7 quoted lines

> Should a check like:
>
>     if(!istate->repo)
>
>         istate->repo = the_repository;
>
> be required?

... if we add such an "if the caller did not set istate->repo, assume the_repository" code, then the resulting code explicitly assumes that the istate the caller passed to us without setting istate->repo belongs to the default repository.

I do not quite see the point of such a change---it is not all that better than "implicit dependency on the_repository" the patch tries to address, is it?