Re: [PATCH 1/2] bisect--helper: `bisect_log` shell function in C
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 16, 2016, 07:36 UTC
- Message-ID
- <CAPig+cTfZ1hWXZcx5OLNRRqJ7FkTigizpMB9E1xzv1Tv+8YY2w@mail.gmail.com>
- In-Reply-To
- <1463169737-12701-1-git-send-email-pranit.bauva@gmail.com>
On Fri, May 13, 2016 at 4:02 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> bisect--helper: `bisect_log` shell function in C
Do you need to insert "rewrite" or "reimplement" in the subject?
Show 14 quoted lines
> 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@gmail.com> > --- > 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.
Show 15 quoted lines
> + 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);
> }