Hi Liam, I made a mistake and accidentally pushed your v4 patch to master a few days ago.
Here is a fix for several issues (warnings, C89 incompatibility, using strpbrk to find end-of-line, append newline to error messages). I removed your extra checks, so that a user can just work with executables in the PATH (posix_spawnp searches the path): wget --use-askpass=ssh-askpass <URL> If the executable does not exists (or something goes wrong when executing), wget will error & stop at that point anyways. Here is the fix, WDYT ? Regards, Tim
From afb66eb94f5e5c4a0cbfed6a1e95d08418bc0dd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim Rühsen?= <[email protected]> Date: Wed, 7 Sep 2016 10:40:43 +0200 Subject: [PATCH] Code cleanup for --use-askpass * bootstrap.conf: Add xmemdup0 and strpbrk. * src/init.c (cmd_use_askpass): Add 'const' to char *, remove check for file existence. * src/main.c (run_use_askpass): C89 compat init of argv, added \n to error messages, fixed stripping of \n and \r from input, make run_use_askpass and use_askpass static. --- bootstrap.conf | 2 ++ src/init.c | 20 +++----------------- src/main.c | 33 ++++++++++++++++++++------------- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index e839277..e706a37 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -90,6 +90,7 @@ stdbool stdint strcase strerror_r-posix +strpbrk strptime strtok_r strtoll @@ -102,6 +103,7 @@ update-copyright vasprintf vsnprintf write +xmemdup0 xstrndup " diff --git a/src/init.c b/src/init.c index dbf356a..b7f573a 100644 --- a/src/init.c +++ b/src/init.c @@ -1388,18 +1388,11 @@ cmd_time (const char *com, const char *val, void *place) static bool cmd_use_askpass (const char *com _GL_UNUSED, const char *val, void *place) { - char *env_name = "WGET_ASKPASS"; - char *env; + const char *env_name = "WGET_ASKPASS"; + const char *env; if (val && *val) - { - if (!file_exists_p (val)) - { - fprintf (stderr, _("%s does not exist.\n"), val); - exit (WGET_EXIT_GENERIC_ERROR); - } - return cmd_string (com, val, place); - } + return cmd_string (com, val, place); env = getenv (env_name); if (!(env && *env)) @@ -1414,13 +1407,6 @@ cmd_use_askpass (const char *com _GL_UNUSED, const char *val, void *place) exit (WGET_EXIT_GENERIC_ERROR); } - if (!file_exists_p (env)) - { - fprintf (stderr, _("%s points to %s, which does not exist.\n"), - env_name, env); - exit (WGET_EXIT_GENERIC_ERROR); - } - return cmd_string (com, env, place); } diff --git a/src/main.c b/src/main.c index 749ec3c..9d0d5d3 100644 --- a/src/main.c +++ b/src/main.c @@ -61,6 +61,7 @@ as that of the covered work. */ #include "version.h" #include "c-strcase.h" #include "dirname.h" +#include "xmemdup0.h" #include <getopt.h> #include <getpass.h> #include <quote.h> @@ -1038,7 +1039,7 @@ prompt_for_password (void) /* Execute external application opt.use_askpass */ -void +static void run_use_askpass (char *question, char **answer) { char tmp[1024]; @@ -1046,12 +1047,12 @@ run_use_askpass (char *question, char **answer) int status; int com[2]; ssize_t bytes = 0; - char * const argv[] = { opt.use_askpass, question, NULL }; + char *argv[3], *p; posix_spawn_file_actions_t fa; if (pipe (com) == -1) { - fprintf (stderr, _("Cannot create pipe")); + fprintf (stderr, _("Cannot create pipe\n")); exit (WGET_EXIT_GENERIC_ERROR); } @@ -1059,7 +1060,7 @@ run_use_askpass (char *question, char **answer) if (status) { fprintf (stderr, - _("Error initializing spawn file actions for use-askpass: %d"), + _("Error initializing spawn file actions for use-askpass: %d\n"), status); exit (WGET_EXIT_GENERIC_ERROR); } @@ -1068,15 +1069,21 @@ run_use_askpass (char *question, char **answer) if (status) { fprintf (stderr, - _("Error setting spawn file actions for use-askpass: %d"), + _("Error setting spawn file actions for use-askpass: %d\n"), status); exit (WGET_EXIT_GENERIC_ERROR); } + /* C89 initializer lists must be computable at load time, + * thus this explicit initialization. */ + argv[0] = opt.use_askpass; + argv[1] = question; + argv[2] = NULL; + status = posix_spawnp (&pid, opt.use_askpass, &fa, NULL, argv, environ); if (status) { - fprintf (stderr, "Error spawning %s: %d", opt.use_askpass, status); + fprintf (stderr, "Error spawning %s: %d\n", opt.use_askpass, status); exit (WGET_EXIT_GENERIC_ERROR); } @@ -1090,19 +1097,19 @@ run_use_askpass (char *question, char **answer) opt.use_askpass, question, strerror (errno)); exit (WGET_EXIT_GENERIC_ERROR); } - /* Set the end byte to \0, and decrement bytes */ - tmp[bytes--] = '\0'; + + /* Make sure there is a trailing 0 */ + tmp[bytes] = '\0'; /* Remove a possible new line */ - while (bytes >= 0 && - (tmp[bytes] == '\0' || tmp[bytes] == '\n' || tmp[bytes] == '\r')) - tmp[bytes--] = '\0'; + if ((p = strpbrk(tmp, "\r\n"))) + bytes = p - tmp; - *answer = xmemdup (tmp, bytes + 2); + *answer = xmemdup0 (tmp, bytes); } /* set the user name and password*/ -void +static void use_askpass (struct url *u) { static char question[1024]; -- 2.9.3
signature.asc
Description: This is a digitally signed message part.
