The function prepare_spawn in windows-spawn.c and os2-spawn.c was a
long-standing memory leak. The first patch fixes the memory leak
and also the dependency on the 'xalloc' module.
Then, the second patch relicenses the module 'windows-spawn' to LGPLv2+.
I am the sole contributor to lib/w32spawn.h and lib/windows-spawn.[hc],
except for
- changes by Paul Eggert (2007-01-26) which are too small to be copyright
relevant,
- changes by Eric Blake (2009-12-05) that were removed on 2020-11-30,
- changes by KO Myung-Hun (2016-01-14) that were moved to os2-spawn.[hc]
on 2020-11-29, and which stay under GPL.
2020-12-10 Bruno Haible <[email protected]>
windows-spawn: Relicense under LGPLv2+.
* modules/windows-spawn (License): Change to LGPLv2+.
2020-12-10 Bruno Haible <[email protected]>
execute, spawn-pipe: Fix memory leak on native Windows.
* lib/windows-spawn.h (prepare_spawn): Add a second parameter.
* lib/windows-spawn.c: Don't include xalloc.h.
(quoted_arg_length, quoted_arg_string): New functions, extracted from
prepare_spawn.
(prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
elements of *new_argv together.
* modules/windows-spawn (Depends-on): Remove xalloc. Add malloc-posix.
* lib/os2-spawn.h (prepare_spawn): Add a second parameter.
* lib/os2-spawn.c: Don't include xalloc.h.
(prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
elements of *new_argv together.
* lib/execute.c: Include xalloc.h.
(execute): Check return value of prepare_spawn. Free the memory
allocated by prepare_spawn.
* modules/execute (Depends-on): Add xalloc-die.
* lib/spawn-pipe.c: Include xalloc.h.
(create_pipe): Check return value of prepare_spawn. Free the memory
allocated by prepare_spawn.
* modules/spawn-pipe (Depends-on): Add xalloc-die.
>From 800920c4b37502e59927464c05c9ca65db0390e3 Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Thu, 10 Dec 2020 23:43:20 +0100
Subject: [PATCH 1/5] execute, spawn-pipe: Fix memory leak on native Windows.
* lib/windows-spawn.h (prepare_spawn): Add a second parameter.
* lib/windows-spawn.c: Don't include xalloc.h.
(quoted_arg_length, quoted_arg_string): New functions, extracted from
prepare_spawn.
(prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
elements of *new_argv together.
* modules/windows-spawn (Depends-on): Remove xalloc. Add malloc-posix.
* lib/os2-spawn.h (prepare_spawn): Add a second parameter.
* lib/os2-spawn.c: Don't include xalloc.h.
(prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
elements of *new_argv together.
* lib/execute.c: Include xalloc.h.
(execute): Check return value of prepare_spawn. Free the memory
allocated by prepare_spawn.
* modules/execute (Depends-on): Add xalloc-die.
* lib/spawn-pipe.c: Include xalloc.h.
(create_pipe): Check return value of prepare_spawn. Free the memory
allocated by prepare_spawn.
* modules/spawn-pipe (Depends-on): Add xalloc-die.
---
ChangeLog | 23 +++++++
lib/execute.c | 15 +++--
lib/os2-spawn.c | 45 ++++++++++---
lib/os2-spawn.h | 2 +-
lib/spawn-pipe.c | 17 +++--
lib/windows-spawn.c | 180 +++++++++++++++++++++++++++++++++-----------------
lib/windows-spawn.h | 10 ++-
modules/execute | 1 +
modules/spawn-pipe | 1 +
modules/windows-spawn | 2 +-
10 files changed, 211 insertions(+), 85 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 89ea7f3..64afab7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,28 @@
2020-12-10 Bruno Haible <[email protected]>
+ execute, spawn-pipe: Fix memory leak on native Windows.
+ * lib/windows-spawn.h (prepare_spawn): Add a second parameter.
+ * lib/windows-spawn.c: Don't include xalloc.h.
+ (quoted_arg_length, quoted_arg_string): New functions, extracted from
+ prepare_spawn.
+ (prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
+ elements of *new_argv together.
+ * modules/windows-spawn (Depends-on): Remove xalloc. Add malloc-posix.
+ * lib/os2-spawn.h (prepare_spawn): Add a second parameter.
+ * lib/os2-spawn.c: Don't include xalloc.h.
+ (prepare_spawn): Use malloc instead of XNMALLOC. Allocate memory for all
+ elements of *new_argv together.
+ * lib/execute.c: Include xalloc.h.
+ (execute): Check return value of prepare_spawn. Free the memory
+ allocated by prepare_spawn.
+ * modules/execute (Depends-on): Add xalloc-die.
+ * lib/spawn-pipe.c: Include xalloc.h.
+ (create_pipe): Check return value of prepare_spawn. Free the memory
+ allocated by prepare_spawn.
+ * modules/spawn-pipe (Depends-on): Add xalloc-die.
+
+2020-12-10 Bruno Haible <[email protected]>
+
findprog-in: Relicense under LGPLv2+.
Paul Smith's approval is in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-12/msg00072.html>.
diff --git a/lib/execute.c b/lib/execute.c
index 03fb6ec..5821e82 100644
--- a/lib/execute.c
+++ b/lib/execute.c
@@ -34,6 +34,7 @@
#include "filename.h"
#include "findprog.h"
#include "wait-process.h"
+#include "xalloc.h"
#include "gettext.h"
#define _(str) gettext (str)
@@ -157,8 +158,11 @@ execute (const char *progname,
/* Native Windows API. */
- /* FIXME: Need to free memory allocated by prepare_spawn. */
- prog_argv = prepare_spawn (prog_argv);
+ char *prog_argv_mem_to_free;
+
+ prog_argv = prepare_spawn (prog_argv, &prog_argv_mem_to_free);
+ if (prog_argv == NULL)
+ xalloc_die ();
int exitcode = -1;
@@ -184,7 +188,7 @@ execute (const char *progname,
HANDLE stderr_handle =
(HANDLE) _get_osfhandle (null_stderr ? nulloutfd : STDERR_FILENO);
- exitcode = spawnpvech (P_WAIT, prog_path, (const char **) prog_argv,
+ exitcode = spawnpvech (P_WAIT, prog_path, (const char **) (prog_argv + 1),
(const char **) environ, directory,
stdin_handle, stdout_handle, stderr_handle);
if (exitcode == -1 && errno == ENOEXEC)
@@ -192,8 +196,7 @@ execute (const char *progname,
/* prog is not a native executable. Try to execute it as a
shell script. Note that prepare_spawn() has already prepended
a hidden element "sh.exe" to prog_argv. */
- prog_argv[0] = prog_path;
- --prog_argv;
+ prog_argv[1] = prog_path;
exitcode = spawnpvech (P_WAIT, prog_argv[0], (const char **) prog_argv,
(const char **) environ, directory,
stdin_handle, stdout_handle, stderr_handle);
@@ -205,6 +208,8 @@ execute (const char *progname,
close (nulloutfd);
if (nullinfd >= 0)
close (nullinfd);
+ free (prog_argv);
+ free (prog_argv_mem_to_free);
free (prog_path_to_free);
if (termsigp != NULL)
diff --git a/lib/os2-spawn.c b/lib/os2-spawn.c
index c15e200..53a35f6 100644
--- a/lib/os2-spawn.c
+++ b/lib/os2-spawn.c
@@ -31,7 +31,6 @@
#include "cloexec.h"
#include "error.h"
-#include "xalloc.h"
#include "gettext.h"
#define _(str) gettext (str)
@@ -94,7 +93,7 @@ undup_safer_noinherit (int tempfd, int origfd)
}
char **
-prepare_spawn (char **argv)
+prepare_spawn (char **argv, char **mem_to_free)
{
size_t argc;
char **new_argv;
@@ -105,24 +104,52 @@ prepare_spawn (char **argv)
;
/* Allocate new argument vector. */
- new_argv = XNMALLOC (1 + argc + 1, char *);
+ new_argv = (char **) malloc ((1 + argc + 1) * sizeof (char *));
+ if (new_argv == NULL)
+ return NULL;
/* Add an element upfront that can be used when argv[0] turns out to be a
script, not a program.
On Unix, this would be "/bin/sh". */
- *new_argv++ = "sh.exe";
+ new_argv[0] = "sh.exe";
/* Put quoted arguments into the new argument vector. */
+ size_t needed_size = 0;
+ for (i = 0; i < argc; i++)
+ {
+ const char *string = argv[i];
+ const char *quoted_string = (string[0] == '\0' ? "\"\"" : string);
+ size_t length = strlen (quoted_string);
+ needed_size += length + 1;
+ }
+
+ char *mem;
+ if (needed_size == 0)
+ mem = NULL;
+ else
+ {
+ mem = (char *) malloc (needed_size);
+ if (mem == NULL)
+ {
+ /* Memory allocation failure. */
+ free (new_argv);
+ errno = ENOMEM;
+ return NULL;
+ }
+ }
+ *mem_to_free = mem;
+
for (i = 0; i < argc; i++)
{
const char *string = argv[i];
- if (string[0] == '\0')
- new_argv[i] = xstrdup ("\"\"");
- else
- new_argv[i] = (char *) string;
+ new_argv[1 + i] = mem;
+ const char *quoted_string = (string[0] == '\0' ? "\"\"" : string);
+ size_t length = strlen (quoted_string);
+ memcpy (mem, quoted_string, length + 1);
+ mem += length + 1;
}
- new_argv[argc] = NULL;
+ new_argv[1 + argc] = NULL;
return new_argv;
}
diff --git a/lib/os2-spawn.h b/lib/os2-spawn.h
index 701cc10..4c8c6a8 100644
--- a/lib/os2-spawn.h
+++ b/lib/os2-spawn.h
@@ -27,6 +27,6 @@ extern int dup_safer_noinherit (int fd);
extern void undup_safer_noinherit (int tempfd, int origfd);
/* Prepares an argument vector before calling spawn(). */
-extern char ** prepare_spawn (char **argv);
+extern char ** prepare_spawn (char **argv, char **mem_to_free);
#endif /* _OS2_SPAWN_H */
diff --git a/lib/spawn-pipe.c b/lib/spawn-pipe.c
index 209bbf5..c73872b 100644
--- a/lib/spawn-pipe.c
+++ b/lib/spawn-pipe.c
@@ -39,6 +39,7 @@
#include "findprog.h"
#include "unistd-safer.h"
#include "wait-process.h"
+#include "xalloc.h"
#include "gettext.h"
#define _(str) gettext (str)
@@ -186,6 +187,7 @@ create_pipe (const char *progname,
using the low-level functions CreatePipe(), DuplicateHandle(),
CreateProcess() and _open_osfhandle(); see the GNU make and GNU clisp
and cvs source code. */
+ char *prog_argv_mem_to_free;
int ifd[2];
int ofd[2];
int child;
@@ -193,8 +195,9 @@ create_pipe (const char *progname,
int stdinfd;
int stdoutfd;
- /* FIXME: Need to free memory allocated by prepare_spawn. */
- prog_argv = prepare_spawn (prog_argv);
+ prog_argv = prepare_spawn (prog_argv, &prog_argv_mem_to_free);
+ if (prog_argv == NULL)
+ xalloc_die ();
if (pipe_stdout)
if (pipe2_safer (ifd, O_BINARY | O_CLOEXEC) < 0)
@@ -278,7 +281,7 @@ create_pipe (const char *progname,
HANDLE stderr_handle =
(HANDLE) _get_osfhandle (null_stderr ? nulloutfd : STDERR_FILENO);
- child = spawnpvech (P_NOWAIT, prog_path, (const char **) prog_argv,
+ child = spawnpvech (P_NOWAIT, prog_path, (const char **) (prog_argv + 1),
(const char **) environ, directory,
stdin_handle, stdout_handle, stderr_handle);
if (child == -1 && errno == ENOEXEC)
@@ -286,8 +289,7 @@ create_pipe (const char *progname,
/* prog is not a native executable. Try to execute it as a
shell script. Note that prepare_spawn() has already prepended
a hidden element "sh.exe" to prog_argv. */
- prog_argv[0] = prog_path;
- --prog_argv;
+ prog_argv[1] = prog_path;
child = spawnpvech (P_NOWAIT, prog_argv[0], (const char **) prog_argv,
(const char **) environ, directory,
stdin_handle, stdout_handle, stderr_handle);
@@ -355,14 +357,13 @@ create_pipe (const char *progname,
but it inherits all open()ed or dup2()ed file handles (which is what
we want in the case of STD*_FILENO). */
{
- child = _spawnvpe (P_NOWAIT, prog_path, (const char **) prog_argv,
+ child = _spawnvpe (P_NOWAIT, prog_path, (const char **) (prog_argv + 1),
(const char **) environ);
if (child == -1 && errno == ENOEXEC)
{
/* prog is not a native executable. Try to execute it as a
shell script. Note that prepare_spawn() has already prepended
a hidden element "sh.exe" to prog_argv. */
- --prog_argv;
child = _spawnvpe (P_NOWAIT, prog_argv[0], (const char **) prog_argv,
(const char **) environ);
}
@@ -390,6 +391,8 @@ create_pipe (const char *progname,
close (ifd[1]);
# endif
+ free (prog_argv);
+ free (prog_argv_mem_to_free);
free (prog_path_to_free);
if (child == -1)
diff --git a/lib/windows-spawn.c b/lib/windows-spawn.c
index 2a59ff2..d252f32 100644
--- a/lib/windows-spawn.c
+++ b/lib/windows-spawn.c
@@ -39,7 +39,6 @@
#include <process.h>
#include "findprog.h"
-#include "xalloc.h"
/* Don't assume that UNICODE is not defined. */
#undef STARTUPINFO
@@ -49,8 +48,82 @@
#define SHELL_SPECIAL_CHARS "\"\\ \001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037*?"
#define SHELL_SPACE_CHARS " \001\002\003\004\005\006\007\010\011\012\013\014\015\016\017\020\021\022\023\024\025\026\027\030\031\032\033\034\035\036\037"
+
+/* Returns the length of a quoted argument string. */
+static size_t
+quoted_arg_length (const char *string)
+{
+ bool quote_around = (strpbrk (string, SHELL_SPACE_CHARS) != NULL);
+ size_t length;
+ unsigned int backslashes;
+ const char *s;
+
+ length = 0;
+ backslashes = 0;
+ if (quote_around)
+ length++;
+ for (s = string; *s != '\0'; s++)
+ {
+ char c = *s;
+ if (c == '"')
+ length += backslashes + 1;
+ length++;
+ if (c == '\\')
+ backslashes++;
+ else
+ backslashes = 0;
+ }
+ if (quote_around)
+ length += backslashes + 1;
+
+ return length;
+}
+
+/* Produces a quoted argument string.
+ Stores exactly quoted_arg_length (STRING) + 1 bytes, including the final
+ NUL byte, at MEM.
+ Returns a pointer past the stored quoted argument string. */
+static char *
+quoted_arg_string (const char *string, char *mem)
+{
+ bool quote_around = (strpbrk (string, SHELL_SPACE_CHARS) != NULL);
+ char *p;
+ unsigned int backslashes;
+ const char *s;
+
+ p = mem;
+ backslashes = 0;
+ if (quote_around)
+ *p++ = '"';
+ for (s = string; *s != '\0'; s++)
+ {
+ char c = *s;
+ if (c == '"')
+ {
+ unsigned int j;
+ for (j = backslashes + 1; j > 0; j--)
+ *p++ = '\\';
+ }
+ *p++ = c;
+ if (c == '\\')
+ backslashes++;
+ else
+ backslashes = 0;
+ }
+ if (quote_around)
+ {
+ unsigned int j;
+ for (j = backslashes; j > 0; j--)
+ *p++ = '\\';
+ *p++ = '"';
+ }
+ *p++ = '\0';
+
+ return p;
+}
+
char **
-prepare_spawn (char **argv)
+prepare_spawn (char **argv, char **mem_to_free)
{
size_t argc;
char **new_argv;
@@ -61,7 +134,7 @@ prepare_spawn (char **argv)
;
/* Allocate new argument vector. */
- new_argv = XNMALLOC (1 + argc + 1, char *);
+ new_argv = (char **) malloc ((1 + argc + 1) * sizeof (char *));
/* Add an element upfront that can be used when argv[0] turns out to be a
script, not a program.
@@ -69,78 +142,63 @@ prepare_spawn (char **argv)
"sh.exe". We have to omit the directory part and rely on the search in
PATH, because the mingw "mount points" are not visible inside Windows
CreateProcess(). */
- *new_argv++ = "sh.exe";
+ new_argv[0] = "sh.exe";
/* Put quoted arguments into the new argument vector. */
+ size_t needed_size = 0;
for (i = 0; i < argc; i++)
{
const char *string = argv[i];
+ size_t length;
if (string[0] == '\0')
- new_argv[i] = xstrdup ("\"\"");
+ length = strlen ("\"\"");
else if (strpbrk (string, SHELL_SPECIAL_CHARS) != NULL)
- {
- bool quote_around = (strpbrk (string, SHELL_SPACE_CHARS) != NULL);
- size_t length;
- unsigned int backslashes;
- const char *s;
- char *quoted_string;
- char *p;
-
- length = 0;
- backslashes = 0;
- if (quote_around)
- length++;
- for (s = string; *s != '\0'; s++)
- {
- char c = *s;
- if (c == '"')
- length += backslashes + 1;
- length++;
- if (c == '\\')
- backslashes++;
- else
- backslashes = 0;
- }
- if (quote_around)
- length += backslashes + 1;
+ length = quoted_arg_length (string);
+ else
+ length = strlen (string);
+ needed_size += length + 1;
+ }
- quoted_string = (char *) xmalloc (length + 1);
+ char *mem;
+ if (needed_size == 0)
+ mem = NULL;
+ else
+ {
+ mem = (char *) malloc (needed_size);
+ if (mem == NULL)
+ {
+ /* Memory allocation failure. */
+ free (new_argv);
+ errno = ENOMEM;
+ return NULL;
+ }
+ }
+ *mem_to_free = mem;
- p = quoted_string;
- backslashes = 0;
- if (quote_around)
- *p++ = '"';
- for (s = string; *s != '\0'; s++)
- {
- char c = *s;
- if (c == '"')
- {
- unsigned int j;
- for (j = backslashes + 1; j > 0; j--)
- *p++ = '\\';
- }
- *p++ = c;
- if (c == '\\')
- backslashes++;
- else
- backslashes = 0;
- }
- if (quote_around)
- {
- unsigned int j;
- for (j = backslashes; j > 0; j--)
- *p++ = '\\';
- *p++ = '"';
- }
- *p = '\0';
+ for (i = 0; i < argc; i++)
+ {
+ const char *string = argv[i];
- new_argv[i] = quoted_string;
+ new_argv[1 + i] = mem;
+ if (string[0] == '\0')
+ {
+ size_t length = strlen ("\"\"");
+ memcpy (mem, "\"\"", length + 1);
+ mem += length + 1;
+ }
+ else if (strpbrk (string, SHELL_SPECIAL_CHARS) != NULL)
+ {
+ mem = quoted_arg_string (string, mem);
}
else
- new_argv[i] = (char *) string;
+ {
+ size_t length = strlen (string);
+ memcpy (mem, string, length + 1);
+ mem += length + 1;
+ }
}
- new_argv[argc] = NULL;
+ new_argv[1 + argc] = NULL;
return new_argv;
}
diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h
index 8801f83..ddbbffa 100644
--- a/lib/windows-spawn.h
+++ b/lib/windows-spawn.h
@@ -25,6 +25,7 @@
#include <windows.h>
/* Prepares an argument vector before calling spawn().
+
Note that spawn() does not by itself call the command interpreter
(getenv ("COMSPEC") != NULL ? getenv ("COMSPEC") :
({ OSVERSIONINFO v; v.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
@@ -53,8 +54,15 @@
- programs that inspect GetCommandLine() and ignore argv,
- mingw programs that have a global variable 'int _CRT_glob = 0;',
- Cygwin programs, when invoked from a Cygwin program.
+
+ prepare_spawn creates and returns a new argument vector, where the arguments
+ are appropriately quoted and an additional argument "sh.exe" has been added
+ at the beginning. The new argument vector is freshly allocated. The memory
+ for all its elements is allocated within *MEM_TO_FREE, which is freshly
+ allocated as well. In case of memory allocation failure, NULL is returned,
+ with errno set.
*/
-extern char ** prepare_spawn (char **argv);
+extern char ** prepare_spawn (char **argv, char **mem_to_free);
/* Creates a subprocess.
MODE is either P_WAIT or P_NOWAIT.
diff --git a/modules/execute b/modules/execute
index cb04003..90a7b89 100644
--- a/modules/execute
+++ b/modules/execute
@@ -32,6 +32,7 @@ stdlib
unistd
wait-process
windows-spawn
+xalloc-die
configure.ac:
gl_EXECUTE
diff --git a/modules/spawn-pipe b/modules/spawn-pipe
index 9d80193..7ad384b 100644
--- a/modules/spawn-pipe
+++ b/modules/spawn-pipe
@@ -40,6 +40,7 @@ unistd
unistd-safer
wait-process
windows-spawn
+xalloc-die
configure.ac:
gl_SPAWN_PIPE
diff --git a/modules/windows-spawn b/modules/windows-spawn
index 4702c50..2538333 100644
--- a/modules/windows-spawn
+++ b/modules/windows-spawn
@@ -13,7 +13,7 @@ stdint
stdlib
strpbrk
unistd
-xalloc
+malloc-posix
configure.ac:
AC_REQUIRE([AC_CANONICAL_HOST])
--
2.7.4
>From d98fb93113776332b51dece972648a4e3fee2fcb Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Thu, 10 Dec 2020 23:49:28 +0100
Subject: [PATCH 2/5] windows-spawn: Relicense under LGPLv2+.
* modules/windows-spawn (License): Change to LGPLv2+.
---
ChangeLog | 5 +++++
modules/windows-spawn | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 64afab7..19fd550 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2020-12-10 Bruno Haible <[email protected]>
+ windows-spawn: Relicense under LGPLv2+.
+ * modules/windows-spawn (License): Change to LGPLv2+.
+
+2020-12-10 Bruno Haible <[email protected]>
+
execute, spawn-pipe: Fix memory leak on native Windows.
* lib/windows-spawn.h (prepare_spawn): Add a second parameter.
* lib/windows-spawn.c: Don't include xalloc.h.
diff --git a/modules/windows-spawn b/modules/windows-spawn
index 2538333..2843921 100644
--- a/modules/windows-spawn
+++ b/modules/windows-spawn
@@ -29,7 +29,7 @@ Include:
"windows-spawn.h"
License:
-GPL
+LGPLv2+
Maintainer:
all
--
2.7.4
