Merge branch 'ds/rev-parse-required-args' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 18 Mar 2014 21:01:05 +0000 (14:01 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 18 Mar 2014 21:01:05 +0000 (14:01 -0700)
"git rev-parse" was loose in rejecting command line arguments that
do not make sense, e.g. "--default" without the required value for
that option.

* ds/rev-parse-required-args:
rev-parse: check i before using argv[i] against argc

1  2 
builtin/rev-parse.c
diff --combined builtin/rev-parse.c
index aaeb611a9729938251b1c679e35c446bb737ff4f,068fd17ac91be66d831cb22461013bc24f9d1c02..45901df37103f37418d7558adb8ace36c8b56aeb
@@@ -9,8 -9,6 +9,8 @@@
  #include "quote.h"
  #include "builtin.h"
  #include "parse-options.h"
 +#include "diff.h"
 +#include "revision.h"
  
  #define DO_REVS               1
  #define DO_NOREV      2
@@@ -32,9 -30,6 +32,9 @@@ static int abbrev_ref
  static int abbrev_ref_strict;
  static int output_sq;
  
 +static int stuck_long;
 +static struct string_list *ref_excludes;
 +
  /*
   * Some arguments are relevant "revision" arguments,
   * others are about output format or other details.
@@@ -190,8 -185,6 +190,8 @@@ static int show_default(void
  
  static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
  {
 +      if (ref_excluded(ref_excludes, refname))
 +              return 0;
        show_rev(NORMAL, sha1, refname);
        return 0;
  }
@@@ -286,7 -279,6 +286,7 @@@ static int try_difference(const char *a
                                exclude = n;
                        }
                }
 +              *dotdot = '.';
                return 1;
        }
        *dotdot = '.';
@@@ -310,10 -302,8 +310,10 @@@ static int try_parent_shorthands(const 
                return 0;
  
        *dotdot = 0;
 -      if (get_sha1_committish(arg, sha1))
 +      if (get_sha1_committish(arg, sha1)) {
 +              *dotdot = '^';
                return 0;
 +      }
  
        if (!parents_only)
                show_rev(NORMAL, sha1, arg);
                show_rev(parents_only ? NORMAL : REVERSED,
                                parents->item->object.sha1, arg);
  
 +      *dotdot = '^';
        return 1;
  }
  
@@@ -331,15 -320,12 +331,15 @@@ static int parseopt_dump(const struct o
        struct strbuf *parsed = o->value;
        if (unset)
                strbuf_addf(parsed, " --no-%s", o->long_name);
 -      else if (o->short_name)
 +      else if (o->short_name && (o->long_name == NULL || !stuck_long))
                strbuf_addf(parsed, " -%c", o->short_name);
        else
                strbuf_addf(parsed, " --%s", o->long_name);
        if (arg) {
 -              strbuf_addch(parsed, ' ');
 +              if (!stuck_long)
 +                      strbuf_addch(parsed, ' ');
 +              else if (o->long_name)
 +                      strbuf_addch(parsed, '=');
                sq_quote_buf(parsed, arg);
        }
        return 0;
@@@ -365,8 -351,6 +365,8 @@@ static int cmd_parseopt(int argc, cons
                OPT_BOOL(0, "stop-at-non-option", &stop_at_non_option,
                                        N_("stop parsing after the "
                                           "first non-option argument")),
 +              OPT_BOOL(0, "stuck-long", &stuck_long,
 +                                      N_("output in stuck long form")),
                OPT_END(),
        };
  
@@@ -492,7 -476,6 +492,7 @@@ N_("git rev-parse --parseopt [options] 
  int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  {
        int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
 +      int has_dashdash = 0;
        int output_prefix = 0;
        unsigned char sha1[20];
        const char *name = NULL;
        if (argc > 1 && !strcmp("-h", argv[1]))
                usage(builtin_rev_parse_usage);
  
 +      for (i = 1; i < argc; i++) {
 +              if (!strcmp(argv[i], "--")) {
 +                      has_dashdash = 1;
 +                      break;
 +              }
 +      }
 +
        prefix = setup_git_directory();
        git_config(git_default_config, NULL);
        for (i = 1; i < argc; i++) {
                        }
                        continue;
                }
 -              if (!prefixcmp(arg, "-n")) {
 +              if (starts_with(arg, "-n")) {
                        if ((filter & DO_FLAGS) && (filter & DO_REVS))
                                show(arg);
                        continue;
                                continue;
                        }
                        if (!strcmp(arg, "--default")) {
-                               def = argv[i+1];
-                               i++;
+                               def = argv[++i];
+                               if (!def)
+                                       die("--default requires an argument");
                                continue;
                        }
                        if (!strcmp(arg, "--prefix")) {
-                               prefix = argv[i+1];
+                               prefix = argv[++i];
+                               if (!prefix)
+                                       die("--prefix requires an argument");
                                startup_info->prefix = prefix;
                                output_prefix = 1;
-                               i++;
                                continue;
                        }
                        if (!strcmp(arg, "--revs-only")) {
                                continue;
                        }
                        if (!strcmp(arg, "--short") ||
 -                          !prefixcmp(arg, "--short=")) {
 +                          starts_with(arg, "--short=")) {
                                filter &= ~(DO_FLAGS|DO_NOREV);
                                verify = 1;
                                abbrev = DEFAULT_ABBREV;
                                symbolic = SHOW_SYMBOLIC_FULL;
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--abbrev-ref") &&
 +                      if (starts_with(arg, "--abbrev-ref") &&
                            (!arg[12] || arg[12] == '=')) {
                                abbrev_ref = 1;
                                abbrev_ref_strict = warn_ambiguous_refs;
                                for_each_ref(show_reference, NULL);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--disambiguate=")) {
 +                      if (starts_with(arg, "--disambiguate=")) {
                                for_each_abbrev(arg + 15, show_abbrev, NULL);
                                continue;
                        }
                                for_each_ref_in("refs/bisect/good", anti_reference, NULL);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--branches=")) {
 +                      if (starts_with(arg, "--branches=")) {
                                for_each_glob_ref_in(show_reference, arg + 11,
                                        "refs/heads/", NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
                        if (!strcmp(arg, "--branches")) {
                                for_each_branch_ref(show_reference, NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--tags=")) {
 +                      if (starts_with(arg, "--tags=")) {
                                for_each_glob_ref_in(show_reference, arg + 7,
                                        "refs/tags/", NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
                        if (!strcmp(arg, "--tags")) {
                                for_each_tag_ref(show_reference, NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--glob=")) {
 +                      if (starts_with(arg, "--glob=")) {
                                for_each_glob_ref(show_reference, arg + 7, NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--remotes=")) {
 +                      if (starts_with(arg, "--remotes=")) {
                                for_each_glob_ref_in(show_reference, arg + 10,
                                        "refs/remotes/", NULL);
 +                              clear_ref_exclusion(&ref_excludes);
                                continue;
                        }
                        if (!strcmp(arg, "--remotes")) {
                                for_each_remote_ref(show_reference, NULL);
 +                              clear_ref_exclusion(&ref_excludes);
 +                              continue;
 +                      }
 +                      if (starts_with(arg, "--exclude=")) {
 +                              add_ref_exclusion(&ref_excludes, arg + 10);
                                continue;
                        }
                        if (!strcmp(arg, "--local-env-vars")) {
                                continue;
                        }
                        if (!strcmp(arg, "--resolve-git-dir")) {
-                               const char *gitdir = resolve_gitdir(argv[i+1]);
+                               const char *gitdir = argv[++i];
                                if (!gitdir)
-                                       die("not a gitdir '%s'", argv[i+1]);
+                                       die("--resolve-git-dir requires an argument");
+                               gitdir = resolve_gitdir(gitdir);
+                               if (!gitdir)
+                                       die("not a gitdir '%s'", argv[i]);
                                puts(gitdir);
                                continue;
                        }
                                                : "false");
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--since=")) {
 +                      if (starts_with(arg, "--since=")) {
                                show_datestring("--max-age=", arg+8);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--after=")) {
 +                      if (starts_with(arg, "--after=")) {
                                show_datestring("--max-age=", arg+8);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--before=")) {
 +                      if (starts_with(arg, "--before=")) {
                                show_datestring("--min-age=", arg+9);
                                continue;
                        }
 -                      if (!prefixcmp(arg, "--until=")) {
 +                      if (starts_with(arg, "--until=")) {
                                show_datestring("--min-age=", arg+8);
                                continue;
                        }
                }
                if (verify)
                        die_no_single_rev(quiet);
 +              if (has_dashdash)
 +                      die("bad revision '%s'", arg);
                as_is = 1;
                if (!show_file(arg, output_prefix))
                        continue;