[PATCH] setup_revisions(): do not access outside argv

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 revision.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 18b7ebb..be1e307 100644
--- a/revision.c
+++ b/revision.c

@@ -1241,9 +1241,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		if (strcmp(arg, "--"))
 			continue;
 		argv[i] = NULL;
-		argc = i;
-		if (argv[i + 1])
+		if (i + 1 < argc && argv[i + 1])
 			revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+		argc = i;
 		seen_dashdash = 1;
 		break;
 	}

-- 
test

Nguyễn Thái Ngọc Duy schrieb:

Why is this necessary? I'd expect that argv arrays have NULL at the end.
If this is a bug, why did noboy notice this earlier? IOW, your commit
message is really lacking justification.

-- Hannes

2009/5/20 Johannes Sixt [off-list ref]:

Nguyễn Thái Ngọc Duy schrieb:

quoted

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 revision.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 18b7ebb..be1e307 100644
--- a/revision.c
+++ b/revision.c

@@ -1241,9 +1241,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch

              if (strcmp(arg, "--"))
                      continue;
              argv[i] = NULL;
-             argc = i;
-             if (argv[i + 1])
+             if (i + 1 < argc && argv[i + 1])
                      revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+             argc = i;
              seen_dashdash = 1;
              break;
      }

Why is this necessary? I'd expect that argv arrays have NULL at the end.

I have no idea. I hit this "bug" in my builtin-rebase.c and had that
question too. But I grepped through and saw that
at least verify_bundle() does not terminate argv with NULL. So I
assume that setup_revisions() does not expect NULL at the end.
-- 
Duy