Hi Pranit,
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> Also reimplement `bisect_voc` shell function in C and call it from
> `bisect_next_check` implementation in C.
Please don't! ;D
> +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...
> +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...
> + bad_syn = xstrdup(bisect_voc("bad"));
> + good_syn = xstrdup(bisect_voc("good"));
...and hence not these two lines...
> + 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...
> + 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]);
...
> + 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.
> + 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.
Cheers
Stephan
