Subject: Re: [PATCH 1/2] dir.c: avoid gcc warning

From: Junio C Hamano <[email-masked]>
Date: Fri, 06 May 2022 13:21:33 -0700

Michael J Gruber <[email-masked]> writes:

> Related to -Wstringop-overread.
>
> In fact, this may be a false positive, but reading until the correct end
> is desirable here anyways.

But the correct end is start + (end - start), not start + (end -
start + 1), isn't it?  We've stripped trailing junk like /.git and
end is point at one byte beyond the end of URL to the repository.

E.g. for "https://auth@host/", we have advanced start to point at
"h" at the beginning of "host", and we have moved end back from
pointing at the NUL at the end to point at "/" at the end of
"host/".

We are trying to make sure that the resulting "host" string between
start and end do not have a slash to apply this special case.

If the original URL were "https://auth@host:4321/", the end points
at "/" at the end of "host:4321/", making the string to be checked
to "host:4321" and we are trying to see it has no '/' in it (which
is the case).  By extending the string by one, memchr() will see the
'/' at the end that is outside.

This seems to be a behaviour breaking change and I am not sure what
we are trying to achieve with it.  Is this a suggestion made by a
broken compiler you have, or something?

Puzzled....

> Signed-off-by: Michael J Gruber <[email-masked]>
> ---
>  dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index 26c4d141ab..32fcaae4c0 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -3145,7 +3145,7 @@ char *git_url_basename(const char *repo, int is_bundle, int is_bare)
>  	 * result in a dir '2222' being guessed due to backwards
>  	 * compatibility.
>  	 */
> -	if (memchr(start, '/', end - start) == NULL
> +	if (memchr(start, '/', end - start + 1) == NULL
>  	    && memchr(start, ':', end - start) != NULL) {
>  		ptr = end;
>  		while (start < ptr && isdigit(ptr[-1]) && ptr[-1] != ':')