Hey Stephan,

Sorry for the late replies. My end semester exams just got over.

On Fri, Nov 18, 2016 at 2:29 AM, Stephan Beyer [off-list ref] wrote:

Hi Pranit,

On 10/14/2016 04:14 PM, Pranit Bauva wrote:

quoted

Also reimplement `bisect_voc` shell function in C and call it from
`bisect_next_check` implementation in C.

Please don't! ;D

quoted

+static char *bisect_voc(char *revision_type)
+{
+     if (!strcmp(revision_type, "bad"))
+             return "bad|new";
+     if (!strcmp(revision_type, "good"))
+             return "good|old";
+
+     return NULL;
+}

Why not simply use something like this:

static const char *voc[] = {
        "bad|new",
        "good|old",
};

Then...

This probably seems a good idea.

quoted

+static int bisect_next_check(const struct bisect_terms *terms,
+                          const char *current_term)
+{
+     int missing_good = 1, missing_bad = 1, retval = 0;
+     char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
+     char *good_glob = xstrfmt("%s-*", terms->term_good);
+     char *bad_syn, *good_syn;

...you don't need bad_syn and good_syn...

quoted

+     bad_syn = xstrdup(bisect_voc("bad"));
+     good_syn = xstrdup(bisect_voc("good"));

...and hence not these two lines...

quoted

+     if (!is_empty_or_missing_file(git_path_bisect_start())) {
+             error(_("You need to give me at least one %s and "
+                     "%s revision. You can use \"git bisect %s\" "
+                     "and \"git bisect %s\" for that. \n"),
+                     bad_syn, good_syn, bad_syn, good_syn);

...and write
                        voc[0], voc[1], voc[0], voc[1]);
instead...

quoted

+             retval = -1;
+             goto finish;
+     }
+     else {
+             error(_("You need to start by \"git bisect start\". You "
+                     "then need to give me at least one %s and %s "
+                     "revision. You can use \"git bisect %s\" and "
+                     "\"git bisect %s\" for that.\n"),
+                     good_syn, bad_syn, bad_syn, good_syn);

...and here
                        voc[1], voc[0], voc[0], voc[1]);
...

quoted

+             retval = -1;
+             goto finish;
+     }
+     goto finish;
+finish:
+     if (!bad_ref)
+             free(bad_ref);
+     if (!good_glob)
+             free(good_glob);
+     if (!bad_syn)
+             free(bad_syn);
+     if (!good_syn)
+             free(good_syn);

...and you can remove the 4 lines above.

quoted

+     return retval;
+}

Besides that, there are again some things that I've already mentioned
and that can be applied here, too, for example, not capitalizing
TERM_GOOD and TERM_BAD, the goto fail simplification, the terms memory leak.

Your suggestion simplifies things, I will use that.

Regards,
Pranit Bauva