git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
Jun 26, 2016, 12:23 UTC
Message-ID
<20160626122356.28999-1-pranit.bauva@gmail.com>
In-Reply-To
<20160607205454.22576-1-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
Previous: Pranit BauvaNext: Pranit Bauva
Message 60 of 75 in “bisect--helper: `bisect_clean_state` shell function in C”
  1. 1/4 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jun 7, 2016
  2. 2/4 t6030: explicitly test for bisection cleanupPranit Bauva, Jun 7, 2016
  3. Eric SunshineJun 7, 2016
  4. Pranit BauvaJun 8, 2016
  5. Eric SunshineJun 8, 2016
  6. Pranit BauvaJun 8, 2016
  7. 3/4 dir: introduce file_size() to check the size of filePranit Bauva, Jun 7, 2016
  8. Eric SunshineJun 8, 2016
  9. Pranit BauvaJun 8, 2016
  10. Eric SunshineJun 8, 2016
  11. Christian CouderJun 8, 2016
  12. Torsten BögershausenJun 8, 2016
  13. Pranit BauvaJun 8, 2016
  14. Torsten BögershausenJun 12, 2016
  15. Pranit BauvaJun 13, 2016
  16. 4/4 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jun 7, 2016
  17. Eric SunshineJun 8, 2016
  18. Christian CouderJun 8, 2016
  19. Christian CouderJun 8, 2016
  20. Eric SunshineJun 8, 2016
  21. Pranit BauvaJun 8, 2016
  22. Eric SunshineJun 8, 2016
  23. Pranit BauvaJun 8, 2016
  24. Eric SunshineJun 7, 2016
  25. Eric SunshineJun 8, 2016
  26. Pranit BauvaJun 8, 2016
  27. Eric SunshineJun 8, 2016
  28. Pranit BauvaJun 8, 2016
  29. Christian CouderJun 8, 2016
  30. Eric SunshineJun 8, 2016
  31. Pranit BauvaJun 8, 2016
  32. 0/6 convert various shell functions in git-bisect to CPranit Bauva, Jun 15, 2016
  33. 1/6 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jun 15, 2016
  34. Eric SunshineJun 15, 2016
  35. Pranit BauvaJun 15, 2016
  36. Eric SunshineJun 15, 2016
  37. 2/6 t6030: explicitly test for bisection cleanupPranit Bauva, Jun 15, 2016
  38. 3/6 wrapper: move is_empty_file() from builtin/am.cPranit Bauva, Jun 15, 2016
  39. Junio C HamanoJun 15, 2016
  40. Pranit BauvaJun 15, 2016
  41. Eric SunshineJun 15, 2016
  42. Pranit BauvaJun 15, 2016
  43. 5/6 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Jun 15, 2016
  44. Eric SunshineJun 15, 2016
  45. Pranit BauvaJun 16, 2016
  46. Eric SunshineJun 16, 2016
  47. Pranit BauvaJun 16, 2016
  48. Christian CouderJun 16, 2016
  49. Pranit BauvaJun 17, 2016
  50. 4/6 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jun 15, 2016
  51. Eric SunshineJun 15, 2016
  52. Pranit BauvaJun 16, 2016
  53. 6/6 bisect--helper: `bisect_write` shell function in CPranit Bauva, Jun 15, 2016
  54. Eric SunshineJun 16, 2016
  55. Pranit BauvaJun 16, 2016
  56. Christian CouderJun 16, 2016
  57. Pranit BauvaJun 17, 2016
  58. Eric SunshineJun 15, 2016
  59. Pranit BauvaJun 15, 2016
  60. 0/6 convert various shell functions in git-bisect to CPranit Bauva, Jun 26, 2016
  61. 1/6 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jun 26, 2016
  62. 2/6 t6030: explicitly test for bisection cleanupPranit Bauva, Jun 26, 2016
  63. 3/6 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Jun 26, 2016
  64. 4/6 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jun 26, 2016
  65. 5/6 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Jun 26, 2016
  66. 6/6 bisect--helper: `bisect_write` shell function in CPranit Bauva, Jun 26, 2016
  67. 0/6 convert various shell functions in git-bisect to CPranit Bauva, Jul 6, 2016
  68. 1/6 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jul 6, 2016
  69. 2/6 t6030: explicitly test for bisection cleanupPranit Bauva, Jul 6, 2016
  70. Junio C HamanoJul 11, 2016
  71. 3/6 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Jul 6, 2016
  72. 4/6 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jul 6, 2016
  73. 5/6 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Jul 6, 2016
  74. 6/6 bisect--helper: `bisect_write` shell function in CPranit Bauva, Jul 6, 2016
  75. Junio C HamanoJul 11, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.