Re: [PATCH v4 4/8] am, rebase--merge: do not overlook --skip'ed commits with post-rewrite

>
> Hi,
>
> On Tue, 11 Dec 2018, Elijah Newren wrote:
>
> > The post-rewrite hook is supposed to be invoked for each rewritten
> > commit.  The fact that a commit was selected and processed by the rebase
> > operation (even though when we hit an error a user said it had no more
> > useful changes), suggests we should write an entry for it.  In
> > particular, let's treat it as an empty commit trivially squashed into
> > its parent.
> >
> > This brings the rebase--am and rebase--merge backends in sync with the
> > behavior of the interactive rebase backend.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
>
> This makes sense. I think, though, that we need to be more careful...
>
> > ---
> >  builtin/am.c                 | 9 +++++++++
> >  git-rebase--merge.sh         | 2 ++
> >  t/t5407-post-rewrite-hook.sh | 3 +++
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/builtin/am.c b/builtin/am.c
> > index 8f27f3375b..af9d034838 100644
> > --- a/builtin/am.c
> > +++ b/builtin/am.c
> > @@ -2000,6 +2000,15 @@ static void am_skip(struct am_state *state)
> >       if (clean_index(&head, &head))
> >               die(_("failed to clean index"));
> >
> > +     if (state->rebasing) {
> > +             FILE *fp = xfopen(am_path(state, "rewritten"), "a");
> > +
> > +             assert(!is_null_oid(&state->orig_commit));
> > +             fprintf(fp, "%s ", oid_to_hex(&state->orig_commit));
>
> ... here. What if `fp == NULL`? (Users do all kinds of interesting
> things...)