Re: [PATCH] rebase: use strvec_pushf() for format-patch revisions
- From
René Scharfe <l.s.r@web.de>
- Date
- Dec 20, 2023, 08:04 UTC
- Message-ID
- <47089803-c45b-4f33-b542-146c313f0902@web.de>
- In-Reply-To
- <xmqqmsu6ce0u.fsf@gitster.g>
Am 19.12.23 um 18:12 schrieb Junio C Hamano:
Show 14 quoted lines
> René Scharfe <l.s.r@web.de> writes: > >> In run_am(), a strbuf is used to create a revision argument that is then >> added to the argument list for git format-patch using strvec_push(). >> Use strvec_pushf() to add it directly instead, simplifying the code. >> >> Signed-off-by: René Scharfe <l.s.r@web.de> >> --- > > Makes sense. Between the location of the original strbuf_addf() > call and the new strvec_pushf() call, nobody mucks with *opts so > this change won't affect the correctness. We no longer use the > extra strbuf, and upon failing to open the rebased-patches file, > we no longer leak the contents of it. Good.
Ha! I didn't even notice that leak on error. Perhaps the two release calls at the end gave me the illusion of cleanliness? Or more likely the opportunity to use strvec_pushf() grabbed my full attention (tunnel vision).
Show 55 quoted lines
>
>> @@ -615,34 +614,32 @@ static int run_am(struct rebase_options *opts)
>> return run_command(&am);
>> }
>>
>> - strbuf_addf(&revisions, "%s...%s",
>> - oid_to_hex(opts->root ?
>> - /* this is now equivalent to !opts->upstream */
>> - &opts->onto->object.oid :
>> - &opts->upstream->object.oid),
>> - oid_to_hex(&opts->orig_head->object.oid));
>> -
>> rebased_patches = xstrdup(git_path("rebased-patches"));
>> format_patch.out = open(rebased_patches,
>> O_WRONLY | O_CREAT | O_TRUNC, 0666);
>> if (format_patch.out < 0) {
>> status = error_errno(_("could not open '%s' for writing"),
>> rebased_patches);
>> free(rebased_patches);
>> strvec_clear(&am.args);
>> return status;
>> }
>>
>> format_patch.git_cmd = 1;
>> strvec_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
>> "--full-index", "--cherry-pick", "--right-only",
>> "--default-prefix", "--no-renames",
>> "--no-cover-letter", "--pretty=mboxrd", "--topo-order",
>> "--no-base", NULL);
>> if (opts->git_format_patch_opt.len)
>> strvec_split(&format_patch.args,
>> opts->git_format_patch_opt.buf);
>> - strvec_push(&format_patch.args, revisions.buf);
>> + strvec_pushf(&format_patch.args, "%s...%s",
>> + oid_to_hex(opts->root ?
>> + /* this is now equivalent to !opts->upstream */
>> + &opts->onto->object.oid :
>> + &opts->upstream->object.oid),
>> + oid_to_hex(&opts->orig_head->object.oid));
>> if (opts->restrict_revision)
>> strvec_pushf(&format_patch.args, "^%s",
>> oid_to_hex(&opts->restrict_revision->object.oid));
>> @@ -665,10 +662,8 @@ static int run_am(struct rebase_options *opts)
>> "As a result, git cannot rebase them."),
>> opts->revisions);
>>
>> - strbuf_release(&revisions);
>> return status;
>> }
>> - strbuf_release(&revisions);
>>
>> am.in = open(rebased_patches, O_RDONLY);
>> if (am.in < 0) {
>> --
>> 2.43.0