Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 10, 2016 at 11:57 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>
> @@ -431,6 +434,244 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
>         return 0;
>  }
>
> +static int bisect_start(struct bisect_terms *terms, int no_checkout,
> +                       const char **argv, int argc)
> +{
> +       int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
> +       int flags, pathspec_pos;
> +       struct string_list revs = STRING_LIST_INIT_DUP;
> +       struct string_list states = STRING_LIST_INIT_DUP;
> +       struct strbuf start_head = STRBUF_INIT;
> +       struct strbuf bisect_names = STRBUF_INIT;
> +       struct strbuf orig_args = STRBUF_INIT;
> +       const char *head;
> +       unsigned char sha1[20];
> +       FILE *fp;
> +       struct object_id oid;
> +
> +       if (is_bare_repository())
> +               no_checkout = 1;
> +
> +       for (i = 0; i < argc; i++) {
> +               char *commit_id = xstrfmt("%s^{commit}", argv[i]);
> +               if (!strcmp(argv[i], "--")) {
> +                       has_double_dash = 1;
> +                       break;
> +               }

In the shell code there is a loop dedicated to checking if there is a
double dash in the arguments before the real argument parsing loop.
There is a reason for that.
If you do it in the same loop, has_double_dash will not be set when
the arguments that are before the double dash will be parsed.

> +               else if (!strcmp(argv[i], "--no-checkout"))
> +                       no_checkout = 1;
> +               else if (!strcmp(argv[i], "--term-good") ||
> +                        !strcmp(argv[i], "--term-old")) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[++i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-good=", &argv[i])) {

(Maybe you could put the "else if (...) {" on the same line as the "}" above.)

> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-old=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (!strcmp(argv[i], "--term-bad") ||
> +                        !strcmp(argv[i], "--term-new")) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_bad);
> +                       strbuf_addstr(&terms->term_bad, argv[++i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-bad=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_bad);
> +                       strbuf_addstr(&terms->term_bad, argv[i]);
> +               }
> +               else if (skip_prefix(argv[i], "--term-new=", &argv[i])) {
> +                       must_write_terms = 1;
> +                       strbuf_reset(&terms->term_good);
> +                       strbuf_addstr(&terms->term_good, argv[i]);
> +               }
> +               else if (starts_with(argv[i], "--") &&
> +                        !one_of(argv[i], "--term-good", "--term-bad", NULL)) {
> +                       string_list_clear(&revs, 0);
> +                       string_list_clear(&states, 0);
> +                       die(_("unrecognised option: '%s'"), argv[i]);
> +               }
> +               else if (get_oid(argv[i], &oid) && !has_double_dash) {

And here checking "!has_double_dash" has no meaning if you check for a
double dash in the same loop, because there is a "break" after
has_double_dash is set above.

> +                       string_list_clear(&revs, 0);
> +                       string_list_clear(&states, 0);
> +                       free(commit_id);
> +                       die(_("'%s' does not appear to be a valid revision"), argv[i]);
> +               }
> +               else {
> +                       free(commit_id);
> +                       string_list_append(&revs, oid_to_hex(&oid));
> +               }
> +       }
> +       pathspec_pos = i;
> +
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]