Re: [PATCH] fetch: fix wrong evaluation order in URL trailing-slash trimming

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2026, 15:36 UTC
Message-ID: <xmqq4in4decx.fsf@gitster.g>
In-Reply-To: <20260225141059.GE2139176@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> I can't help but think this would be easier to read without an empty
> loop body, like:
>
>   for (len = strlen(display_state->url); len > 0; len--) {
> 	if (display_state->url[len-1] != '/')
> 		break;
>   }
>
> which makes it much more clear we never leave the bounds of the string
> (and also works with a size_t, which is a more appropriate type than int
> here).

Yes, this is vastly more readable, even though what it does is exactly the same as the original.

Or instead of having strlen() to scan the entire string once and then ourselves scan backwards from the end, scan forward ourselves only once while noting where the last non-slash byte was, or something.

> But it may not be worth polishing this bit of code too much (if we did,
> I'd also suggest strip_suffix_mem() to drop ".git" rather than all of
> those magic numbers. Or even stuffing it in a strbuf and using
> strbuf_setlen() and strbuf_strip_suffix().

True. None of these "we could do it this way too" bikeshedding has much value. The patch posted is an obvious and trivial enough improvement.

Thanks.