@matheusbsbs / matheus-bagliano / commits / 850d2fec53

convert manual allocations to argv_array

There are many manual argv allocations that predate the argv_array API. Switching to that API brings a few advantages: 1. We no longer have to manually compute the correct final array size (so it's one less thing we can screw up). 2. In many cases we had to make a separate pass to count, then allocate, then fill in the array. Now we can do it in one pass, making the code shorter and easier to follow. 3. argv_array handles memory ownership for us, making it more obvious when things should be free()d and and when not. Most of these cases are pretty straightforward. In some, we switch from "run_command_v" to "run_command" which lets us directly use the argv_array embedded in "struct child_process". 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 850d2fec53ee188bab9e458f77906041ac7f1904

7 files changed +43 -76

builtin/grep.c

+5 -5

@@ -354,17 +354,17 @@ static void append_path(struct grep_opt *opt, const void *data, size_t len)

354 static void run_pager(struct grep_opt *opt, const char *prefix)

355 {

356 struct string_list *path_list = opt->output_priv;

357 - const char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1));

357 + struct child_process child = CHILD_PROCESS_INIT;

358 int i, status;

359

360 for (i = 0; i < path_list->nr; i++)

361 - argv[i] = path_list->items[i].string;

362 - argv[path_list->nr] = NULL;

361 + argv_array_push(&child.args, path_list->items[i].string);

362 + child.dir = prefix;

363 + child.use_shell = 1;

364

364 - status = run_command_v_opt_cd_env(argv, RUN_USING_SHELL, prefix, NULL);

365 + status = run_command(&child);

366 if (status)

367 exit(status);

367 - free(argv);

368 }

369

370 static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int cached)

builtin/receive-pack.c

+3 -9

@@ -1031,7 +1031,6 @@ static void run_update_post_hook(struct command *commands)

1031 {

1032 struct command *cmd;

1033 int argc;

1034 - const char **argv;

1034 struct child_process proc = CHILD_PROCESS_INIT;

1035 const char *hook;

1036

@@ -1044,21 +1043,16 @@ static void run_update_post_hook(struct command *commands)

1043 if (!argc || !hook)

1044 return;

1045

1047 - argv = xmalloc(sizeof(*argv) * (2 + argc));

1048 - argv[0] = hook;

1049 -

1050 - for (argc = 1, cmd = commands; cmd; cmd = cmd->next) {

1046 + argv_array_push(&proc.args, hook);

1047 + for (cmd = commands; cmd; cmd = cmd->next) {

1048 if (cmd->error_string || cmd->did_not_exist)

1049 continue;

1053 - argv[argc] = xstrdup(cmd->ref_name);

1054 - argc++;

1050 + argv_array_push(&proc.args, cmd->ref_name);

1051 }

1056 - argv[argc] = NULL;

1052

1053 proc.no_stdin = 1;

1054 proc.stdout_to_stderr = 1;

1055 proc.err = use_sideband ? -1 : 0;

1061 - proc.argv = argv;

1056

1057 if (!start_command(&proc)) {

1058 if (use_sideband)

builtin/remote-ext.c

+5 -21

@@ -114,30 +114,14 @@ static char *strip_escapes(const char *str, const char *service,

114 }

115 }

116

117 -/* Should be enough... */

118 -#define MAXARGUMENTS 256

119 -

120 -static const char **parse_argv(const char *arg, const char *service)

117 +static void parse_argv(struct argv_array *out, const char *arg, const char *service)

118 {

122 - int arguments = 0;

123 - int i;

124 - const char **ret;

125 - char *temparray[MAXARGUMENTS + 1];

126 -

119 while (*arg) {

128 - char *expanded;

129 - if (arguments == MAXARGUMENTS)

130 - die("remote-ext command has too many arguments");

131 - expanded = strip_escapes(arg, service, &arg);

120 + char *expanded = strip_escapes(arg, service, &arg);

121 if (expanded)

133 - temparray[arguments++] = expanded;

122 + argv_array_push(out, expanded);

123 + free(expanded);

124 }

135 -

136 - ret = xmalloc((arguments + 1) * sizeof(char *));

137 - for (i = 0; i < arguments; i++)

138 - ret[i] = temparray[i];

139 - ret[arguments] = NULL;

140 - return ret;

125 }

126

127 static void send_git_request(int stdin_fd, const char *serv, const char *repo,

@@ -158,7 +142,7 @@ static int run_child(const char *arg, const char *service)

142 child.in = -1;

143 child.out = -1;

144 child.err = 0;

161 - child.argv = parse_argv(arg, service);

145 + parse_argv(&child.args, arg, service);

146

147 if (start_command(&child) < 0)

148 die("Can't run specified command");

daemon.c

+5 -7

@@ -808,7 +808,7 @@ static void check_dead_children(void)

808 cradle = &blanket->next;

809 }

810

811 -static char **cld_argv;

811 +static struct argv_array cld_argv = ARGV_ARRAY_INIT;

812 static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)

813 {

814 struct child_process cld = CHILD_PROCESS_INIT;

@@ -842,7 +842,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)

842 #endif

843 }

844

845 - cld.argv = (const char **)cld_argv;

845 + cld.argv = cld_argv.argv;

846 cld.in = incoming;

847 cld.out = dup(incoming);

848

@@ -1374,12 +1374,10 @@ int main(int argc, char **argv)

1374 write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());

1375

1376 /* prepare argv for serving-processes */

1377 - cld_argv = xmalloc(sizeof (char *) * (argc + 2));

1378 - cld_argv[0] = argv[0]; /* git-daemon */

1379 - cld_argv[1] = "--serve";

1377 + argv_array_push(&cld_argv, argv[0]); /* git-daemon */

1378 + argv_array_push(&cld_argv, "--serve");

1379 for (i = 1; i < argc; ++i)

1381 - cld_argv[i+1] = argv[i];

1382 - cld_argv[argc+1] = NULL;

1380 + argv_array_push(&cld_argv, argv[i]);

1381

1382 return serve(&listen_addr, listen_port, cred);

1383 }

git.c

+5 -9

@@ -239,19 +239,15 @@ static int handle_alias(int *argcp, const char ***argv)

239 alias_string = alias_lookup(alias_command);

240 if (alias_string) {

241 if (alias_string[0] == '!') {

242 - const char **alias_argv;

243 - int argc = *argcp, i;

242 + struct child_process child = CHILD_PROCESS_INIT;

243

244 commit_pager_choice();

245

247 - /* build alias_argv */

248 - alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));

249 - alias_argv[0] = alias_string + 1;

250 - for (i = 1; i < argc; ++i)

251 - alias_argv[i] = (*argv)[i];

252 - alias_argv[argc] = NULL;

246 + child.use_shell = 1;

247 + argv_array_push(&child.args, alias_string + 1);

248 + argv_array_pushv(&child.args, (*argv) + 1);

249

254 - ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);

250 + ret = run_command(&child);

251 if (ret >= 0) /* normal exit */

252 exit(ret);

253

line-log.c

+9 -13

@@ -14,6 +14,7 @@

14 #include "graph.h"

15 #include "userdiff.h"

16 #include "line-log.h"

17 +#include "argv-array.h"

19 static void range_set_grow(struct range_set *rs, size_t extra)

20 {

@@ -746,22 +747,17 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list

747 add_line_range(rev, commit, range);

748

749 if (!rev->diffopt.detect_rename) {

749 - int i, count = 0;

750 - struct line_log_data *r = range;

750 + struct line_log_data *r;

751 + struct argv_array array = ARGV_ARRAY_INIT;

752 const char **paths;

752 - while (r) {

753 - count++;

754 - r = r->next;

755 - }

756 - paths = xmalloc((count+1)*sizeof(char *));

757 - r = range;

758 - for (i = 0; i < count; i++) {

759 - paths[i] = xstrdup(r->path);

760 - r = r->next;

761 - }

762 - paths[count] = NULL;

753 +

754 + for (r = range; r; r = r->next)

755 + argv_array_push(&array, r->path);

756 + paths = argv_array_detach(&array);

757 +

758 parse_pathspec(&rev->diffopt.pathspec, 0,

759 PATHSPEC_PREFER_FULL, "", paths);

760 + /* strings are now owned by pathspec */

761 free(paths);

762 }

763 }

remote-curl.c

+11 -12

@@ -845,23 +845,22 @@ static void parse_fetch(struct strbuf *buf)

845

846 static int push_dav(int nr_spec, char **specs)

847 {

848 - const char **argv = xmalloc((10 + nr_spec) * sizeof(char*));

849 - int argc = 0, i;

848 + struct child_process child = CHILD_PROCESS_INIT;

849 + size_t i;

850

851 - argv[argc++] = "http-push";

852 - argv[argc++] = "--helper-status";

851 + child.git_cmd = 1;

852 + argv_array_push(&child.args, "http-push");

853 + argv_array_push(&child.args, "--helper-status");

854 if (options.dry_run)

854 - argv[argc++] = "--dry-run";

855 + argv_array_push(&child.args, "--dry-run");

856 if (options.verbosity > 1)

856 - argv[argc++] = "--verbose";

857 - argv[argc++] = url.buf;

857 + argv_array_push(&child.args, "--verbose");

858 + argv_array_push(&child.args, url.buf);

859 for (i = 0; i < nr_spec; i++)

859 - argv[argc++] = specs[i];

860 - argv[argc++] = NULL;

860 + argv_array_push(&child.args, specs[i]);

861

862 - if (run_command_v_opt(argv, RUN_GIT_CMD))

863 - die("git-%s failed", argv[0]);

864 - free(argv);

862 + if (run_command(&child))

863 + die("git-http-push failed");

864 return 0;

865 }

866