Subject: Re: [PATCH/RFD] fix connection via git protocol

From: Junio C Hamano <[email-masked]>
Date: Mon, 17 Apr 2023 09:33:57 -0700

Jeff King <[email-masked]> writes:

> The preprocessor doesn't know that we'll be confused if "s" isn't
> resolved, and by the time the compiler sees it, it's a string already.
>
> Obviously we could add a test that catches this at run-time, but we
> should be able to do better (catch it earlier, and with less code).

Ah, great minds think alike.  I am glad you have already thought it
through.

> My first thought was: why can't we just treat the port as an "int" in
> the first place? The answer is mostly that getaddrinfo() expects it as a
> string. It could even be a non-numeric service like "http" in theory
> (and looked up in /etc/services; Debian's even has "git" in it!), but
> our get_host_and_port() refuses to allow that. But even if we didn't
> want to ever support non-numeric service names, it makes the code more
> awkward (we have to format the port into an extra buffer).
>
> This would work:
>
> diff --git a/connect.c b/connect.c
> index fd3179e545..1eba71e34c 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -753,7 +753,7 @@ static char *host_end(char **hoststart, int removebrackets)
>  }
>  
>  #define STR_(s)	# s
> -#define STR(s)	STR_(s)
> +#define STR(s) (STR_(s) + BUILD_ASSERT_OR_ZERO(s))

OOoooh.  Clever.  A pointer plus N indexes into an array, but if the
offset is N then the pointer is left intact so the caller does not
see the difference.

> ... But the
> BUILD_ASSERT doesn't seem too bad to me.

Indeed.