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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.