Thread (227 messages) 227 messages, 10 authors, 2019-01-07

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help