prepare_{git,shell}_cmd: use argv_array
These functions transform an existing argv into one suitable for exec-ing or spawning via git or a shell. We can use an argv_array in each to avoid dealing with manual counting and allocation. This also makes the memory allocation more clear and fixes some leaks. In prepare_shell_cmd, we would sometimes allocate a new string with "$@" in it and sometimes not, meaning the caller could not correctly free it. On the non-Windows side, we are in a child process which will exec() or exit() immediately, so the leak isn't a big deal. On Windows, though, we use spawn() from the parent process, and leak a string for each shell command we run. On top of that, the Windows code did not free the allocated argv array at all (but does for the prepare_git_cmd case!). By switching both of these functions to write into an argv_array, we can consistently free the result as appropriate. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Feb 22, 2016 at 17:44 UTC
20574f551bcc5fcf0f0e20236af174754fa11363
3 files changed
+39
-53
exec_cmd.c
+11
-17
@@ -1,6 +1,7 @@
1
#include "cache.h"
2
#include "exec_cmd.h"
3
#include "quote.h"
4
+#include "argv-array.h"
5
#define MAX_ARGS 32
6
7
static const char *argv_exec_path;
@@ -107,32 +108,25 @@ void setup_path(void)
108
strbuf_release(&new_path);
109
}
110
110
-const char **prepare_git_cmd(const char **argv)
111
+const char **prepare_git_cmd(struct argv_array *out, const char **argv)
112
{
112
- int argc;
113
- const char **nargv;
114
-
115
- for (argc = 0; argv[argc]; argc++)
116
- ; /* just counting */
117
- nargv = xmalloc(sizeof(*nargv) * (argc + 2));
118
-
119
- nargv[0] = "git";
120
- for (argc = 0; argv[argc]; argc++)
121
- nargv[argc + 1] = argv[argc];
122
- nargv[argc + 1] = NULL;
123
- return nargv;
113
+ argv_array_push(out, "git");
114
+ argv_array_pushv(out, argv);
115
+ return out->argv;
116
}
117
118
int execv_git_cmd(const char **argv) {
127
- const char **nargv = prepare_git_cmd(argv);
128
- trace_argv_printf(nargv, "trace: exec:");
119
+ struct argv_array nargv = ARGV_ARRAY_INIT;
120
+
121
+ prepare_git_cmd(&nargv, argv);
122
+ trace_argv_printf(nargv.argv, "trace: exec:");
123
124
/* execvp() can only ever return if it fails */
131
- sane_execvp("git", (char **)nargv);
125
+ sane_execvp("git", (char **)nargv.argv);
126
127
trace_printf("trace: exec failed: %s\n", strerror(errno));
128
135
- free(nargv);
129
+ argv_array_clear(&nargv);
130
return -1;
131
}
132
exec_cmd.h
+3
-1
@@ -1,11 +1,13 @@
1
#ifndef GIT_EXEC_CMD_H
2
#define GIT_EXEC_CMD_H
3
4
+struct argv_array;
5
+
6
extern void git_set_argv_exec_path(const char *exec_path);
7
extern const char *git_extract_argv0_path(const char *path);
8
extern const char *git_exec_path(void);
9
extern void setup_path(void);
8
-extern const char **prepare_git_cmd(const char **argv);
10
+extern const char **prepare_git_cmd(struct argv_array *out, const char **argv);
11
extern int execv_git_cmd(const char **argv); /* NULL terminated */
12
LAST_ARG_MUST_BE_NULL
13
extern int execl_git_cmd(const char *cmd, ...);
run-command.c
+25
-35
@@ -158,50 +158,41 @@ int sane_execvp(const char *file, char * const argv[])
158
return -1;
159
}
160
161
-static const char **prepare_shell_cmd(const char **argv)
161
+static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
162
{
163
- int argc, nargc = 0;
164
- const char **nargv;
165
-
166
- for (argc = 0; argv[argc]; argc++)
167
- ; /* just counting */
168
- /* +1 for NULL, +3 for "sh -c" plus extra $0 */
169
- nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3));
170
-
171
- if (argc < 1)
163
+ if (!argv[0])
164
die("BUG: shell command is empty");
165
166
if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
167
#ifndef GIT_WINDOWS_NATIVE
176
- nargv[nargc++] = SHELL_PATH;
168
+ argv_array_push(out, SHELL_PATH);
169
#else
178
- nargv[nargc++] = "sh";
170
+ argv_array_push(out, "sh");
171
#endif
180
- nargv[nargc++] = "-c";
181
-
182
- if (argc < 2)
183
- nargv[nargc++] = argv[0];
184
- else {
185
- struct strbuf arg0 = STRBUF_INIT;
186
- strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
187
- nargv[nargc++] = strbuf_detach(&arg0, NULL);
188
- }
189
- }
172
+ argv_array_push(out, "-c");
173
191
- for (argc = 0; argv[argc]; argc++)
192
- nargv[nargc++] = argv[argc];
193
- nargv[nargc] = NULL;
174
+ /*
175
+ * If we have no extra arguments, we do not even need to
176
+ * bother with the "$@" magic.
177
+ */
178
+ if (!argv[1])
179
+ argv_array_push(out, argv[0]);
180
+ else
181
+ argv_array_pushf(out, "%s \"$@\"", argv[0]);
182
+ }
183
195
- return nargv;
184
+ argv_array_pushv(out, argv);
185
+ return out->argv;
186
}
187
188
#ifndef GIT_WINDOWS_NATIVE
189
static int execv_shell_cmd(const char **argv)
190
{
201
- const char **nargv = prepare_shell_cmd(argv);
202
- trace_argv_printf(nargv, "trace: exec:");
203
- sane_execvp(nargv[0], (char **)nargv);
204
- free(nargv);
191
+ struct argv_array nargv = ARGV_ARRAY_INIT;
192
+ prepare_shell_cmd(&nargv, argv);
193
+ trace_argv_printf(nargv.argv, "trace: exec:");
194
+ sane_execvp(nargv.argv[0], (char **)nargv.argv);
195
+ argv_array_clear(&nargv);
196
return -1;
197
}
198
#endif
@@ -455,6 +446,7 @@ fail_pipe:
446
{
447
int fhin = 0, fhout = 1, fherr = 2;
448
const char **sargv = cmd->argv;
449
+ struct argv_array nargv = ARGV_ARRAY_INIT;
450
451
if (cmd->no_stdin)
452
fhin = open("/dev/null", O_RDWR);
@@ -480,9 +472,9 @@ fail_pipe:
472
fhout = dup(cmd->out);
473
474
if (cmd->git_cmd)
483
- cmd->argv = prepare_git_cmd(cmd->argv);
475
+ cmd->argv = prepare_git_cmd(&nargv, cmd->argv);
476
else if (cmd->use_shell)
485
- cmd->argv = prepare_shell_cmd(cmd->argv);
477
+ cmd->argv = prepare_shell_cmd(&nargv, cmd->argv);
478
479
cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env,
480
cmd->dir, fhin, fhout, fherr);
@@ -492,9 +484,7 @@ fail_pipe:
484
if (cmd->clean_on_exit && cmd->pid >= 0)
485
mark_child_for_cleanup(cmd->pid);
486
495
- if (cmd->git_cmd)
496
- free(cmd->argv);
497
-
487
+ argv_array_clear(&nargv);
488
cmd->argv = sargv;
489
if (fhin != 0)
490
close(fhin);