[PATCH v3 0/6] convert various shell functions in git-bisect to C

[Pranit Bauva <pranit.bauva@gmail.com>]

The major change introduced in this version is that I have used `struct
bisect_terms` to store term_good and term_bad and then I am passing around
the memory address of it to functions. Also I have made various changes
in accordance with the previous review.

Here is the link for v2.
http://thread.gmane.org/gmane.comp.version-control.git/297372

Thanks for suggestions Eric Sunshine, Christian Couder and Junio C Hammano.

Here's the interdiff:
diff --git a/builtin/am.c b/builtin/am.c
index 84f21d0..6ee158f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1307,7 +1307,7 @@ static int parse_mail(struct am_state *state, const char *mail)
 		goto finish;
 	}
 
-	if (is_empty_file(am_path(state, "patch"))) {
+	if (is_empty_or_missing_file(am_path(state, "patch"))) {
 		printf_ln(_("Patch is empty. Was it split wrong?"));
 		die_user_resolve(state);
 	}
@@ -1895,7 +1895,7 @@ next:
 		resume = 0;
 	}
 
-	if (!is_empty_file(am_path(state, "rewritten"))) {
+	if (!is_empty_or_missing_file(am_path(state, "rewritten"))) {
 		assert(state->rebasing);
 		copy_notes_for_rebase(state);
 		run_post_rewrite_hook(state);
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index eebfcf0..e946ba9 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -26,6 +26,25 @@ static const char * const git_bisect_helper_usage[] = {
 	NULL
 };
 
+struct bisect_terms {
+	struct strbuf term_good;
+	struct strbuf term_bad;
+};
+
+static int bisect_terms_init(struct bisect_terms *term)
+{
+	strbuf_init(&term->term_good, 0);
+	strbuf_init(&term->term_bad, 0);
+	return 0;
+}
+
+static int bisect_terms_release(struct bisect_terms *term)
+{
+	strbuf_release(&term->term_good);
+	strbuf_release(&term->term_good);
+	return 0;
+}
+
 /*
  * Check whether the string `term` belongs to the set of strings
  * included in the variable arguments.
@@ -110,6 +129,7 @@ static int bisect_clean_state(void)
 	for_each_ref_in("refs/bisect/", mark_for_removal, (void *) &refs_for_removal);
 	string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
 	result = delete_refs(&refs_for_removal);
+	refs_for_removal.strdup_strings = 1;
 	string_list_clear(&refs_for_removal, 0);
 	remove_path(git_path_bisect_expected_rev());
 	remove_path(git_path_bisect_ancestors_ok());
@@ -138,12 +158,11 @@ static int bisect_reset(const char *commit)
 			return 0;
 		}
 		strbuf_rtrim(&branch);
-
 	} else {
 		struct object_id oid;
 		if (get_oid(commit, &oid))
 			return error(_("'%s' is not a valid commit"), commit);
-		strbuf_addf(&branch, "%s", commit);
+		strbuf_addstr(&branch, commit);
 	}
 
 	if (!file_exists(git_path_bisect_head())) {
@@ -151,7 +170,7 @@ static int bisect_reset(const char *commit)
 		argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
 		if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
 			error(_("Could not check out original HEAD '%s'. Try"
-					"'git bisect reset <commit>'."), branch.buf);
+				"'git bisect reset <commit>'."), branch.buf);
 			strbuf_release(&branch);
 			argv_array_clear(&argv);
 			return -1;
@@ -166,15 +185,11 @@ static int bisect_reset(const char *commit)
 static int is_expected_rev(const char *expected_hex)
 {
 	struct strbuf actual_hex = STRBUF_INIT;
-	int res;
-
-	if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) < 0) {
-		strbuf_release(&actual_hex);
-		return 0;
+	int res = 0;
+	if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 0) {
+		strbuf_trim(&actual_hex);
+		res = !strcmp(actual_hex.buf, expected_hex);
 	}
-
-	strbuf_trim(&actual_hex);
-	res = !strcmp(actual_hex.buf, expected_hex);
 	strbuf_release(&actual_hex);
 	return res;
 }
@@ -194,8 +209,7 @@ static int check_expected_revs(const char **revs, int rev_nr)
 }
 
 static int bisect_write(const char *state, const char *rev,
-			const char *term_good, const char *term_bad,
-			int nolog)
+			const struct bisect_terms *term, int nolog)
 {
 	struct strbuf tag = STRBUF_INIT;
 	struct strbuf commit_name = STRBUF_INIT;
@@ -204,9 +218,9 @@ static int bisect_write(const char *state, const char *rev,
 	struct pretty_print_context pp = {0};
 	FILE *fp;
 
-	if (!strcmp(state, term_bad))
+	if (!strcmp(state, term->term_bad.buf))
 		strbuf_addf(&tag, "refs/bisect/%s", state);
-	else if(one_of(state, term_good, "skip", NULL))
+	else if(one_of(state, term->term_good.buf, "skip", NULL))
 		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
 	else
 		return error(_("Bad bisect_write argument: %s"), state);
@@ -221,23 +235,21 @@ static int bisect_write(const char *state, const char *rev,
 		strbuf_release(&tag);
 		return -1;
 	}
+	strbuf_release(&tag);
 
 	fp = fopen(git_path_bisect_log(), "a");
-	if (!fp) {
-		strbuf_release(&tag);
+	if (!fp)
 		return error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
-	}
 
 	commit = lookup_commit_reference(oid.hash);
 	format_commit_message(commit, "%s", &commit_name, &pp);
 	fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
 		commit_name.buf);
+	strbuf_release(&commit_name);
 
 	if (!nolog)
 		fprintf(fp, "git bisect %s %s\n", state, rev);
 
-	strbuf_release(&commit_name);
-	strbuf_release(&tag);
 	fclose(fp);
 	return 0;
 }
@@ -252,7 +264,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		CHECK_EXPECTED_REVS,
 		BISECT_WRITE
 	} cmdmode = 0;
-	int no_checkout = 0;
+	int no_checkout = 0, res = 0;
 	struct option options[] = {
 		OPT_CMDMODE(0, "next-all", &cmdmode,
 			 N_("perform 'git bisect next'"), NEXT_ALL),
@@ -270,6 +282,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
 	};
+	struct bisect_terms state;
+	bisect_terms_init(&state);
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
@@ -284,24 +298,32 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	case WRITE_TERMS:
 		if (argc != 2)
 			die(_("--write-terms requires two arguments"));
-		return write_terms(argv[0], argv[1]);
+		res = write_terms(argv[0], argv[1]);
+		break;
 	case BISECT_CLEAN_STATE:
 		if (argc != 0)
 			die(_("--bisect-clean-state requires no arguments"));
-		return bisect_clean_state();
+		res = bisect_clean_state();
+		break;
 	case BISECT_RESET:
 		if (argc > 1)
 			die(_("--bisect-reset requires either zero or one arguments"));
-		return bisect_reset(argc ? argv[0] : NULL);
+		res = bisect_reset(argc ? argv[0] : NULL);
+		break;
 	case CHECK_EXPECTED_REVS:
-		return check_expected_revs(argv, argc);
+		res = check_expected_revs(argv, argc);
+		break;
 	case BISECT_WRITE:
 		if (argc != 4 && argc != 5)
 			die(_("--bisect-write requires either 4 or 5 arguments"));
 		nolog = (argc == 5) && !strcmp(argv[4], "nolog");
-		return bisect_write(argv[0], argv[1], argv[2], argv[3], nolog);
+		strbuf_addstr(&state.term_good, argv[2]);
+		strbuf_addstr(&state.term_bad, argv[3]);
+		res = bisect_write(argv[0], argv[1], &state, nolog);
+		break;
 	default:
 		die("BUG: unknown subcommand '%d'", cmdmode);
 	}
-	return 0;
+	bisect_terms_release(&state);
+	return res;
 }
diff --git a/cache.h b/cache.h
index 8eaad70..91e2f81 100644
--- a/cache.h
+++ b/cache.h
@@ -1871,6 +1871,6 @@ void sleep_millisec(int millisec);
 void safe_create_dir(const char *dir, int share);
 
 /* Return 1 if the file is empty or does not exists, 0 otherwise. */
-extern int is_empty_file(const char *filename);
+extern int is_empty_or_missing_file(const char *filename);
 
 #endif /* CACHE_H */
diff --git a/wrapper.c b/wrapper.c
index 36a3eeb..e70e4d1 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -697,14 +697,14 @@ void sleep_millisec(int millisec)
 	poll(NULL, 0, millisec);
 }
 
-int is_empty_file(const char *filename)
+int is_empty_or_missing_file(const char *filename)
 {
 	struct stat st;
 
 	if (stat(filename, &st) < 0) {
 		if (errno == ENOENT)
 			return 1;
-		error_errno(_("could not stat %s"), filename);
+		die_errno(_("could not stat %s"), filename);
 	}
 
 	return !st.st_size;



Pranit Bauva (6):
  bisect--helper: `bisect_clean_state` shell function in C
  t6030: explicitly test for bisection cleanup
  wrapper: move is_empty_file() and rename it as
    is_empty_or_missing_file()
  bisect--helper: `bisect_reset` shell function in C
  bisect--helper: `is_expected_rev` & `check_expected_revs` shell
    function in C
  bisect--helper: `bisect_write` shell function in C

 builtin/am.c                |  20 +---
 builtin/bisect--helper.c    | 222 +++++++++++++++++++++++++++++++++++++++++++-
 cache.h                     |   3 +
 git-bisect.sh               |  97 ++-----------------
 t/t6030-bisect-porcelain.sh |  17 ++++
 wrapper.c                   |  13 +++
 6 files changed, 263 insertions(+), 109 deletions(-)

-- 
2.9.0