setup_revisions(): take pathspec from command line and --stdin correctly

When the command line has "--" disambiguator, we take the remainder of argv[] as "prune_data", but when --stdin is given at the same time, we need to append to the existing prune_data and end up attempting to realloc(3) it. That would not work. Fix it by consistently using append_prune_data() throughout the input processing. Also avoid counting the number of existing paths in the function over and over again. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed May 11, 2011 at 14:01 UTC 4da5af3151301d25a25d60a90b8275eaca13de86
2 files changed +45 -52
revision.c
+28 -52
@@ -953,35 +953,34 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
953 return 0;
954 }
955
956 -static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb, const char ***prune_data)
957 -{
958 - const char **prune = *prune_data;
959 - int prune_nr;
960 - int prune_alloc;
956 +struct cmdline_pathspec {
957 + int alloc;
958 + int nr;
959 + const char **path;
960 +};
961
962 - /* count existing ones */
963 - if (!prune)
964 - prune_nr = 0;
965 - else
966 - for (prune_nr = 0; prune[prune_nr]; prune_nr++)
967 - ;
968 - prune_alloc = prune_nr; /* not really, but we do not know */
962 +static void append_prune_data(struct cmdline_pathspec *prune, const char **av)
963 +{
964 + while (*av) {
965 + ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
966 + prune->path[prune->nr++] = *(av++);
967 + }
968 +}
969
970 +static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
971 + struct cmdline_pathspec *prune)
972 +{
973 while (strbuf_getwholeline(sb, stdin, '\n') != EOF) {
974 int len = sb->len;
975 if (len && sb->buf[len - 1] == '\n')
976 sb->buf[--len] = '\0';
974 - ALLOC_GROW(prune, prune_nr+1, prune_alloc);
975 - prune[prune_nr++] = xstrdup(sb->buf);
977 + ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
978 + prune->path[prune->nr++] = xstrdup(sb->buf);
979 }
977 - if (prune) {
978 - ALLOC_GROW(prune, prune_nr+1, prune_alloc);
979 - prune[prune_nr] = NULL;
980 - }
981 - *prune_data = prune;
980 }
981
984 -static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune)
982 +static void read_revisions_from_stdin(struct rev_info *revs,
983 + struct cmdline_pathspec *prune)
984 {
985 struct strbuf sb;
986 int seen_dashdash = 0;
@@ -1267,34 +1266,6 @@ static int for_each_good_bisect_ref(each_ref_fn fn, void *cb_data)
1266 return for_each_ref_in("refs/bisect/good", fn, cb_data);
1267 }
1268
1270 -static void append_prune_data(const char ***prune_data, const char **av)
1271 -{
1272 - const char **prune = *prune_data;
1273 - int prune_nr;
1274 - int prune_alloc;
1275 -
1276 - if (!prune) {
1277 - *prune_data = av;
1278 - return;
1279 - }
1280 -
1281 - /* count existing ones */
1282 - for (prune_nr = 0; prune[prune_nr]; prune_nr++)
1283 - ;
1284 - prune_alloc = prune_nr; /* not really, but we do not know */
1285 -
1286 - while (*av) {
1287 - ALLOC_GROW(prune, prune_nr+1, prune_alloc);
1288 - prune[prune_nr++] = *av;
1289 - av++;
1290 - }
1291 - if (prune) {
1292 - ALLOC_GROW(prune, prune_nr+1, prune_alloc);
1293 - prune[prune_nr] = NULL;
1294 - }
1295 - *prune_data = prune;
1296 -}
1297 -
1269 /*
1270 * Parse revision information, filling in the "rev_info" structure,
1271 * and removing the used arguments from the argument list.
@@ -1305,7 +1276,9 @@ static void append_prune_data(const char ***prune_data, const char **av)
1276 int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
1277 {
1278 int i, flags, left, seen_dashdash, read_from_stdin;
1308 - const char **prune_data = NULL;
1279 + struct cmdline_pathspec prune_data;
1280 +
1281 + memset(&prune_data, 0, sizeof(prune_data));
1282
1283 /* First, search for "--" */
1284 seen_dashdash = 0;
@@ -1316,7 +1289,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
1289 argv[i] = NULL;
1290 argc = i;
1291 if (argv[i + 1])
1319 - prune_data = argv + i + 1;
1292 + append_prune_data(&prune_data, argv + i + 1);
1293 seen_dashdash = 1;
1294 break;
1295 }
@@ -1408,8 +1381,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
1381 }
1382 }
1383
1411 - if (prune_data)
1412 - revs->prune_data = get_pathspec(revs->prefix, prune_data);
1384 + if (prune_data.nr) {
1385 + ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
1386 + prune_data.path[prune_data.nr++] = NULL;
1387 + revs->prune_data = get_pathspec(revs->prefix, prune_data.path);
1388 + }
1389
1390 if (revs->def == NULL)
1391 revs->def = def;
t/t6017-rev-list-stdin.sh
+17
@@ -58,4 +58,21 @@ check side-3 ^side-4 -- file-3
58 check side-3 ^side-2
59 check side-3 ^side-2 -- file-1
60
61 +test_expect_success 'not only --stdin' '
62 + cat >expect <<-EOF &&
63 + 7
64 +
65 + file-1
66 + file-2
67 + EOF
68 + cat >input <<-EOF &&
69 + ^master^
70 + --
71 + file-2
72 + EOF
73 + git log --pretty=tformat:%s --name-only --stdin master -- file-1 \
74 + <input >actual &&
75 + test_cmp expect actual
76 +'
77 +
78 test_done