Hi David, Q: I'm a little confused by the motivation ? why does adding that '--url' before the <server> argument really make anything easier? A: The value here is the ability to define <server> in a config file. That's the sole purpose of the patch. I added a new argument "--url" for this. Specifying both --url and <server> does not make sense, but still allowed - did not want to put unnecessary restrictions and to keep backward compatibility. Defining <server> in config allows to use syntax like "openconnect --config=XXX" to run openconnect, which in turn is useful for portability and for systemd units, allowing to set up openconnect similarly to openvpn: https://wiki.archlinux.org/index.php/OpenVPN#systemd_service_configuration It makes sense to also ship a .service file similar to openvpn, but that would be a separate commit (and I'd need to figure out where to put such file in the repo...). Here is the patch once again: ############################################ commit 52e665b7bf8e8f08d78a36e84ff67c0ef5183ee1 Author: Dmitrii Sutiagin <f3flight at gmail.com> Date: Thu May 18 12:49:40 2017 -0700 Add URL config option This new option allows putting <server> argument into a config file, which in turn allows cleaner setups of systemd units / automation. If --url is used then argument can be omitted. If both --url and <server> are specified then <server> is used. Signed-off-by: Dmitrii Sutiagin <f3flight at gmail.com> diff --git a/main.c b/main.c index 7869f72..eab4f41 100644 --- a/main.c +++ b/main.c @@ -188,6 +188,7 @@ enum { OPT_LOCAL_HOSTNAME, OPT_PROTOCOL, OPT_PASSTOS, + OPT_URL, }; #ifdef __sun__ @@ -269,6 +270,7 @@ static const struct option long_options[] = { OPTION("dump-http-traffic", 0, OPT_DUMP_HTTP), OPTION("no-system-trust", 0, OPT_NO_SYSTEM_TRUST), OPTION("protocol", 1, OPT_PROTOCOL), + OPTION("url", 1, OPT_URL), #ifdef OPENCONNECT_GNUTLS OPTION("gnutls-debug", 1, OPT_GNUTLS_DEBUG), #endif @@ -867,6 +869,7 @@ static void usage(void) printf(" --resolve=HOST:IP %s\n", _("Use IP when connecting to HOST")); printf(" --os=STRING %s\n", _("OS type (linux,linux-64,win,...) to report")); printf(" --dtls-local-port=PORT %s\n", _("Set local port for DTLS datagrams")); + printf(" --url=URL %s\n", _("Set URL to connect to. If set, <server> argument can be omitted")); print_supported_protocols_usage(); printf("\n"); @@ -1068,6 +1071,7 @@ int main(int argc, char **argv) oc_token_mode_t token_mode = OC_TOKEN_MODE_NONE; int reconnect_timeout = 300; int ret; + char *url = NULL; #ifdef HAVE_NL_LANGINFO char *charset; #endif @@ -1431,6 +1435,9 @@ int main(int argc, char **argv) case OPT_TIMESTAMP: timestamp = 1; break; + case OPT_URL: + url = keep_config_arg(); + break; #ifdef OPENCONNECT_GNUTLS case OPT_GNUTLS_DEBUG: gnutls_global_set_log_level(atoi(config_arg)); @@ -1448,9 +1455,12 @@ int main(int argc, char **argv) if (optind < argc - 1) { fprintf(stderr, _("Too many arguments on command line\n")); usage(); - } else if (optind > argc - 1) { - fprintf(stderr, _("No server specified\n")); - usage(); + } else if (optind == argc - 1) { + url = strdup(argv[optind]); + } + if (!(url && strlen(url))) { + fprintf(stderr, _("No URL or server specified\n")); + usage(); } if (!vpninfo->sslkey) @@ -1501,16 +1511,12 @@ int main(int argc, char **argv) if (vpninfo->sslkey && do_passphrase_from_fsid) openconnect_passphrase_from_fsid(vpninfo); - if (config_lookup_host(vpninfo, argv[optind])) + if (config_lookup_host(vpninfo, url)) exit(1); if (!vpninfo->hostname) { - char *url = strdup(argv[optind]); - if (openconnect_parse_url(vpninfo, url)) exit(1); - - free(url); } /* Historically, the path in the URL superseded the one in the ############################################ On 05/18/2017 01:34 PM, David Woodhouse wrote: > On Thu, 2017-05-18 at 13:00 -0700, Dmitry Sutyagin wrote: >> Hi everyone, I wanted to contribute with the following patch: >> >> ############################################ >> commit 52e665b7bf8e8f08d78a36e84ff67c0ef5183ee1 >> Author: Dmitrii Sutiagin <f3flight at gmail.com> >> Date: Thu May 18 12:49:40 2017 -0700 >> >> Add URL config option >> >> This new option allows putting argument into a config file, >> which in turn allows cleaner setups of systemd units / automation. >> If --url is used then argument can be omitted. >> If both --url and are specified then is used. >> >> Signed-off-by: Dmitrii Sutiagin <f3flight at gmail.com> > > Hi Dmitry, thanks for the patch. Unfortunately it was whitespace- > damaged so won't apply. Can you try sending it from a better mail > client? You can send it to yourself first, then save it to a file and > try to apply what you receive. > > I'm a little confused by the motivation ? why does adding that '--url' > before the <server> argument really make anything easier? >
