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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html