On Thu, Oct 23, 2025 at 05:37:25PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

On Tue, Oct 21, 2025 at 08:48:31PM -0700, Junio C Hamano wrote:

quoted

quoted

Practically speaking that's probably OK, since we are unlikely to have
so many active paths anyway (or if we did, we'd likely have other
problems to deal with ;-)), but it is gross nonetheless.

The case path_idx() returns -1 is an error case, not "there are too
many paths we are following" case.  I do not see what relevance the
number of active paths has here.

I just meant that we are unlikely to ever have so many active paths at
once that (size_t)-1 would actually have a valid entry, or IOW that
active_paths_nr is smaller than 2^32-1.

So?  If path_idx() needed to signal an error, it will return (size_t)-1,
but as the compiler correctly caught, the code as written, i.e.

	k = path_idx(...);
	if (0 <= k) {
		/* did not error so we can safely use k */
		...
	}

is outright buggy. I do not see why it is "practically speaking
that's probably OK".  It certainly does not matter if the number we
will receive in 'k' in the success case is expected to be
small---the problem is only for an error case.

I am not saying that we should continue to write "if (0 <= k)", since we
will clearly never take the else branch. I am trying to say that
path_idx() *could* return (size_t)-1, and callers would be able to write
"if (k == (size_t)-1)" to check for that error condition.

My observation was that there are unlikely to be so many active paths at
any one time such that we'd ever want to return (size_t)-1 as a valid
index, and could always use it as an error sentinel.

Thanks,
Taylor