connect.c: stop conflating ssh command names and overrides

dd33e07766 ("connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config", 2017-02-01) attempted to add support for configuration and environment variable to override the different handling of port_option and needs_batch settings suitable for variants of the ssh implementation that was autodetected by looking at the ssh command name. Because it piggybacked on the code that turns command name to specific override (e.g. "plink.exe" and "plink" means port_option needs to be set to 'P' instead of the default 'p'), yet it defined a separate namespace for these overrides (e.g. "putty" can be usable to signal that port_option needs to be 'P'), however, it made the auto-detection based on the command name less robust (e.g. the code now accepts "putty" as a SSH command name and applies the same override). Separate the code that interprets the override that was read from the configuration & environment from the original code that handles the command names, as they are in separate namespaces, to fix this confusion. This incidentally also makes it easier for future enhancement of the override syntax (e.g. "port_option=p,needs_batch=1" may want to be accepted as a more explicit syntax) without affecting the code for auto-detection based on the command name. While at it, update the return type of the handle_ssh_variant() helper function to void; the caller does not use it, and the function does not return any meaningful value. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Feb 9, 2017 at 09:20 UTC 486c8e8c6a42a1e0537eedb2b5ab9e74eb58d5f7
1 file changed +32 -13
connect.c
+32 -13
@@ -691,17 +691,39 @@ static const char *get_ssh_command(void)
691 return NULL;
692 }
693
694 -static int handle_ssh_variant(const char *ssh_command, int is_cmdline,
695 - int *port_option, int *needs_batch)
694 +static int override_ssh_variant(int *port_option, int *needs_batch)
695 {
697 - const char *variant = getenv("GIT_SSH_VARIANT");
696 + char *variant;
697 +
698 + variant = xstrdup_or_null(getenv("GIT_SSH_VARIANT"));
699 + if (!variant &&
700 + git_config_get_string("ssh.variant", &variant))
701 + return 0;
702 +
703 + if (!strcmp(variant, "plink") || !strcmp(variant, "putty")) {
704 + *port_option = 'P';
705 + *needs_batch = 0;
706 + } else if (!strcmp(variant, "tortoiseplink")) {
707 + *port_option = 'P';
708 + *needs_batch = 1;
709 + } else {
710 + *port_option = 'p';
711 + *needs_batch = 0;
712 + }
713 + free(variant);
714 + return 1;
715 +}
716 +
717 +static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
718 + int *port_option, int *needs_batch)
719 +{
720 + const char *variant;
721 char *p = NULL;
722
700 - if (variant)
701 - ; /* okay, fall through */
702 - else if (!git_config_get_string("ssh.variant", &p))
703 - variant = p;
704 - else if (!is_cmdline) {
723 + if (override_ssh_variant(port_option, needs_batch))
724 + return;
725 +
726 + if (!is_cmdline) {
727 p = xstrdup(ssh_command);
728 variant = basename(p);
729 } else {
@@ -717,12 +739,11 @@ static int handle_ssh_variant(const char *ssh_command, int is_cmdline,
739 */
740 free(ssh_argv);
741 } else
720 - return 0;
742 + return;
743 }
744
745 if (!strcasecmp(variant, "plink") ||
724 - !strcasecmp(variant, "plink.exe") ||
725 - !strcasecmp(variant, "putty"))
746 + !strcasecmp(variant, "plink.exe"))
747 *port_option = 'P';
748 else if (!strcasecmp(variant, "tortoiseplink") ||
749 !strcasecmp(variant, "tortoiseplink.exe")) {
@@ -730,8 +751,6 @@ static int handle_ssh_variant(const char *ssh_command, int is_cmdline,
751 *needs_batch = 1;
752 }
753 free(p);
733 -
734 - return 1;
754 }
755
756 /*