Re: [PATCH v15 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
From: Stephan Beyer <hidden>
Date: 2016-11-17 20:59:55
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