On Fri, Apr 14, 2023 at 7:23 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote:
>
> (I was able to get a segfault by rebasing this series with
> --author=stolee because the commit list became empty. Something
> to watch for.)
>
> > Something like:
> >
> > 'git replay [options] <base> <tip>'
> > This mode means to rebase <tip> onto <base>,
> > detecting the range of commits to rewrite.
> >
> > 'git replay [options] <new-base> <old-base> <tip>'
> > This mode means to rebase the range <old-base>..<tip>
> > onto <new-base>.
>
> For that reason, I think we should be using explicit argument
> parsing in the builtin and only transform arguments we
> understand into the setup_revisions() (by building a strvec).
So, it turns out that this suggested solution wouldn't have helped
prevent the segfault you found. If someone merely passed <old-base>
== <tip>, they'd see the same segfault. In fact, I think you found a
latent bug in merge-ort. In particular, cmd_replay() has
struct merge_options merge_opt;
struct merge_result result;
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
<do N merges, for some value of N>
merge_finalize(&merge_opt, &result);
This code segfaults if N is 0, because merge_finalize() doesn't return
early when result->priv is NULL.
We never triggered this before, because this is the very first code
example we've had where someone has tried to call merge_finalize()
without first performing a merge. Anyway, nice find and thanks for
reporting!
