Skip to content

Commit 7fa3c2a

Browse files
peffgitster
authored andcommitted
revision: replace "struct cmdline_pathspec" with argv_array
We assemble an array of strings in a custom struct, NULL-terminate the result, and then pass it to parse_pathspec(). But then we never free the array or the individual strings (nor can we do the latter, as they are heap-allocated when they come from stdin but not when they come from the passed-in argv). Let's swap this out for an argv_array. It does the same thing with fewer lines of code, and it's safe to call argv_array_clear() at the end to avoid a memory leak. Reported-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 9ddaf86 commit 7fa3c2a

1 file changed

Lines changed: 11 additions & 28 deletions

File tree

revision.c

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "bisect.h"
2222
#include "packfile.h"
2323
#include "worktree.h"
24+
#include "argv-array.h"
2425

2526
volatile show_early_output_fn_t show_early_output;
2627

@@ -1672,31 +1673,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
16721673
return 0;
16731674
}
16741675

1675-
struct cmdline_pathspec {
1676-
int alloc;
1677-
int nr;
1678-
const char **path;
1679-
};
1680-
1681-
static void append_prune_data(struct cmdline_pathspec *prune, const char **av)
1682-
{
1683-
while (*av) {
1684-
ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
1685-
prune->path[prune->nr++] = *(av++);
1686-
}
1687-
}
1688-
16891676
static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
1690-
struct cmdline_pathspec *prune)
1677+
struct argv_array *prune)
16911678
{
1692-
while (strbuf_getline(sb, stdin) != EOF) {
1693-
ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
1694-
prune->path[prune->nr++] = xstrdup(sb->buf);
1695-
}
1679+
while (strbuf_getline(sb, stdin) != EOF)
1680+
argv_array_push(prune, sb->buf);
16961681
}
16971682

16981683
static void read_revisions_from_stdin(struct rev_info *revs,
1699-
struct cmdline_pathspec *prune)
1684+
struct argv_array *prune)
17001685
{
17011686
struct strbuf sb;
17021687
int seen_dashdash = 0;
@@ -2286,10 +2271,9 @@ static void NORETURN diagnose_missing_default(const char *def)
22862271
int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)
22872272
{
22882273
int i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0, revarg_opt;
2289-
struct cmdline_pathspec prune_data;
2274+
struct argv_array prune_data = ARGV_ARRAY_INIT;
22902275
const char *submodule = NULL;
22912276

2292-
memset(&prune_data, 0, sizeof(prune_data));
22932277
if (opt)
22942278
submodule = opt->submodule;
22952279

@@ -2305,7 +2289,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
23052289
argv[i] = NULL;
23062290
argc = i;
23072291
if (argv[i + 1])
2308-
append_prune_data(&prune_data, argv + i + 1);
2292+
argv_array_pushv(&prune_data, argv + i + 1);
23092293
seen_dashdash = 1;
23102294
break;
23112295
}
@@ -2366,14 +2350,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
23662350
for (j = i; j < argc; j++)
23672351
verify_filename(revs->prefix, argv[j], j == i);
23682352

2369-
append_prune_data(&prune_data, argv + i);
2353+
argv_array_pushv(&prune_data, argv + i);
23702354
break;
23712355
}
23722356
else
23732357
got_rev_arg = 1;
23742358
}
23752359

2376-
if (prune_data.nr) {
2360+
if (prune_data.argc) {
23772361
/*
23782362
* If we need to introduce the magic "a lone ':' means no
23792363
* pathspec whatsoever", here is the place to do so.
@@ -2388,11 +2372,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
23882372
* call init_pathspec() to set revs->prune_data here.
23892373
* }
23902374
*/
2391-
ALLOC_GROW(prune_data.path, prune_data.nr + 1, prune_data.alloc);
2392-
prune_data.path[prune_data.nr++] = NULL;
23932375
parse_pathspec(&revs->prune_data, 0, 0,
2394-
revs->prefix, prune_data.path);
2376+
revs->prefix, prune_data.argv);
23952377
}
2378+
argv_array_clear(&prune_data);
23962379

23972380
if (revs->def == NULL)
23982381
revs->def = opt ? opt->def : NULL;

0 commit comments

Comments
 (0)