[PATCH] 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, which would not work and can lead to a segfault or worse.

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 <redacted>
---

 * This is relative to 1.6.6; the bug originates back to 60da8b1 (Make
  --stdin option to "log" family read also pathspecs, 2009-11-20).

 revision.c                |   80 ++++++++++++++++-----------------------------
 t/t6017-rev-list-stdin.sh |   17 +++++++++
 2 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/revision.c b/revision.c
index c92ffc2..58b5651 100644
--- a/revision.c
+++ b/revision.c

@@ -956,35 +956,34 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,

       return 0;
 }

-static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb, const char ***prune_data)
-{
-       const char **prune = *prune_data;
-       int prune_nr;
-       int prune_alloc;
+struct cmdline_pathspec {

+               prune->path[prune->nr++] = *(av++);
+       }
+}

+static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
+                                    struct cmdline_pathspec *prune)
+{
       while (strbuf_getwholeline(sb, stdin, '\n') != EOF) {
               int len = sb->len;
               if (len && sb->buf[len - 1] == '\n')
                       sb->buf[--len] = '\0';
-               ALLOC_GROW(prune, prune_nr+1, prune_alloc);
-               prune[prune_nr++] = xstrdup(sb->buf);
+               ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
+               prune->path[prune->nr++] = xstrdup(sb->buf);
       }
-       if (prune) {
-               ALLOC_GROW(prune, prune_nr+1, prune_alloc);
-               prune[prune_nr] = NULL;
-       }
-       *prune_data = prune;
 }

-static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune)
+static void read_revisions_from_stdin(struct rev_info *revs,
+                                     struct cmdline_pathspec *prune)
 {
       struct strbuf sb;
       int seen_dashdash = 0;

@@ -1279,34 +1278,6 @@ static int for_each_good_bisect_ref(each_ref_fn fn, void *cb_data)

       return for_each_ref_in("refs/bisect/good", fn, cb_data);
 }

-static void append_prune_data(const char ***prune_data, const char **av)
-{
-       const char **prune = *prune_data;
-       int prune_nr;
-       int prune_alloc;
-
-       if (!prune) {
-               *prune_data = av;
-               return;
-       }
-
-       /* count existing ones */
-       for (prune_nr = 0; prune[prune_nr]; prune_nr++)
-               ;
-       prune_alloc = prune_nr; /* not really, but we do not know */
-
-       while (*av) {
-               ALLOC_GROW(prune, prune_nr+1, prune_alloc);
-               prune[prune_nr++] = *av;
-               av++;
-       }
-       if (prune) {
-               ALLOC_GROW(prune, prune_nr+1, prune_alloc);
-               prune[prune_nr] = NULL;
-       }
-       *prune_data = prune;
-}
-
 /*
 * Parse revision information, filling in the "rev_info" structure,
 * and removing the used arguments from the argument list.

@@ -1317,7 +1288,9 @@ static void append_prune_data(const char ***prune_data, const char **av)

 int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
 {
       int i, flags, left, seen_dashdash, read_from_stdin;
-       const char **prune_data = NULL;
+       struct cmdline_pathspec prune_data;
+
+       memset(&prune_data, 0, sizeof(prune_data));

       /* First, search for "--" */
       seen_dashdash = 0;

@@ -1328,7 +1301,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch

               argv[i] = NULL;
               argc = i;
               if (argv[i + 1])
-                       prune_data = argv + i + 1;
+                       append_prune_data(&prune_data, argv + i + 1);
               seen_dashdash = 1;
               break;
       }

@@ -1420,8 +1393,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch

               }
       }

-       if (prune_data)
-               revs->prune_data = get_pathspec(revs->prefix, prune_data);
+       if (prune_data.nr) {
+               ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
+               prune_data.path[prune_data.nr++] = NULL;
+               revs->prune_data = get_pathspec(revs->prefix, prune_data.path);
+       }

       if (revs->def == NULL)
               revs->def = def;

diff --git a/t/t6017-rev-list-stdin.sh b/t/t6017-rev-list-stdin.sh
index f1c32db..667b375 100755
--- a/t/t6017-rev-list-stdin.sh
+++ b/t/t6017-rev-list-stdin.sh

@@ -58,4 +58,21 @@ check side-3 ^side-4 -- file-3

 check side-3 ^side-2
 check side-3 ^side-2 -- file-1

+test_expect_success 'not only --stdin' '
+       cat >expect <<-EOF &&
+       7
+
+       file-1
+       file-2
+       EOF
+       cat >input <<-EOF &&
+       ^master^
+       --
+       file-2
+       EOF
+       git log --pretty=tformat:%s --name-only --stdin master -- file-1 \
+               <input >actual &&
+       test_cmp expect actual
+'
+
 test_done
--
1.7.5.1.315.ge7efa

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html