On Fri, May 13, 2016 at 4:02 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: > bisect--helper: `bisect_log` shell function in C Do you need to insert "rewrite" or "reimplement" in the subject? > Reimplement the `bisect_log` shell function in C and add a > `--bisect-log` subcommand to `git bisect--helper` to call it from > git-bisect.sh . > > Using `--bisect-log` subcommand is a temporary measure to port shell > function to C so as to use the existing test suite. As more functions > are ported, this subcommand will be retired and will be called by some > other method. > > Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> > --- > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > @@ -79,11 +80,26 @@ int write_terms(const char *bad, const char *good) > +int bisect_log(void) s/^/static/ > +{ > + struct strbuf buf = STRBUF_INIT; > + > + if (strbuf_read_file(&buf, ".git/BISECT_LOG", 256) < 0) As mentioned in my review of the "write-terms" rewrite, hardcoding ".git/" here is wrong for a variety of reasons. See get_git_dir(), get_git_common_dir(), etc. in cache.h instead. > + return error(_("We are not bisecting.")); > + > + printf("%s", buf.buf); > + strbuf_release(&buf); > + > + return 0; > +} > + > int cmd_bisect__helper(int argc, const char **argv, const char *prefix) > { > @@ -109,6 +127,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) > if (argc != 2) > die(_("--write-terms requires two arguments")); > return write_terms(argv[0], argv[1]); > + case BISECT_LOG: Shouldn't you be die()ing here with an appropriate error message if argc is not 0? > + return bisect_log(); > default: > die("BUG: unknown subcommand '%d'", cmdmode); > } -- 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