Re: [PATCH v2 2/6] rebase -i: remove patch file after conflict resolution

From: Glen Choo <chooglen@google.com>
Date: Jun 21, 2023, 20:14 UTC
Message-ID: <kl6lsfakr2sr.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <227aea031b588977f22f3f97faee981d79ade05c.1682089074.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> @@ -3490,7 +3495,6 @@ static int make_patch(struct repository *r,
>  		return -1;
>  	res |= write_rebase_head(&commit->object.oid);
>  
> -	strbuf_addf(&buf, "%s/patch", get_dir(opts));
>  	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
>  	repo_init_revisions(r, &log_tree_opt, NULL);
>  	log_tree_opt.abbrev = 0;

I was checking to see if we could remove buf or whether we are reusing it for unrelated reasons (which is a common Git-ism). We can't remove it because we reuse it, however...

Show 9 quoted lines

> @@ -3498,7 +3502,7 @@ static int make_patch(struct repository *r,
>  	log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
>  	log_tree_opt.disable_stdin = 1;
>  	log_tree_opt.no_commit_id = 1;
> -	log_tree_opt.diffopt.file = fopen(buf.buf, "w");
> +	log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
>  	log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
>  	if (!log_tree_opt.diffopt.file)
>  		res |= error_errno(_("could not open '%s'"), buf.buf);

this buf.buf was supposed to be the value we populated earlier - this should be rebase_path_patch() instead.

As an aside, I have a mild distaste the Git-ism of reusing "struct strbuf buf" - using a variable for just a single purpose and naming it as such makes these sorts of errors much easier to spot. That isn't something we need to fix here, I'm just venting a little :)