Re: [PATCH v2] ssh: add 'ssh.keyfile' option

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 23, 2020, 19:38 UTC
Message-ID: <xmqqeesegfgy.fsf@gitster.c.googlers.com>
In-Reply-To: <AM0PR04MB47715A57B110E14D6631EACDA5D30@AM0PR04MB4771.eurprd04.prod.outlook.com>

Matthias Aßhauer <mha1993@live.de> writes:

Show 7 quoted lines

>>+		switch (variant) {
>>+		case VARIANT_AUTO:
>>+			BUG("VARIANT_AUTO passed to push_ssh_options");
>
> Why do we keep replicating this all over push_ssh_options? The wording
> very much sounds like it's a bug whenever VARIANT_AUTO gets passed to
> push_ssh_options, no exceptions. In that case ...

My reading of the code tells me that it is *not* the case. When the caller asks to connect only over IPv4 with CONNECT_IPV4 bit set in the flags, we cannot translate that to an option to underlying SSH implementation without knowing the variant. As long as the caller does not specify IPV4/IPV6 or custom port, I think it is OK for the caller to leave the variant AUTO.

So you probably want a patch along the lines of...

 connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/connect.c b/connect.c
index b6451ab5e8..b02cf74c9a 100644
--- a/connect.c
+++ b/connect.c
@@ -1118,7 +1118,7 @@ static void push_ssh_options(struct argv_array *args, struct argv_array *env,
 	if (flags & CONNECT_IPV4) {
 		switch (variant) {
 		case VARIANT_AUTO:
-			BUG("VARIANT_AUTO passed to push_ssh_options");
+			BUG("VARIANT_AUTO and CONNECT_IPV4 used together");
 		case VARIANT_SIMPLE:
 			die(_("ssh variant 'simple' does not support -4"));
 		case VARIANT_SSH: