Re: [PATCH 2/6] Split unpack_trees 'reset' flag into two for untracked handling

[Elijah Newren <newren@gmail.com>]

On Sun, Sep 19, 2021 at 6:51 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Sat, Sep 18 2021, Elijah Newren via GitGitGadget wrote:
>
> > -     opts.reset = reset;
> > +     opts.reset_keep_untracked = reset;
> >       opts.fn = twoway_merge;
> > +     /* Setup opts.dir so that ignored files in the way get overwritten */
> > +     opts.dir = xcalloc(1, sizeof(*opts.dir));
> > +     opts.dir->flags |= DIR_SHOW_IGNORED;
> > +     setup_standard_excludes(opts.dir);
>
> Is the "opts.dir" free'd later somehow?

No, much like other callsites that set this up (e.g.
builtin/checkout.c), there isn't a place that frees it.  In copying
how they worked, I also copied their bugs.  ;-)

I'm tempted to move this code into setup_unpack_trees_porcelain() and
then free it in clear_unpack_trees_porcelain()...though not all
callers make use of those functions.  Hmm...

> >       opts.head_idx = -1;
> >       opts.update = worktree;
> >       opts.skip_unmerged = !worktree;
> > -     opts.reset = 1;
> > +     if (o->force)
> > +             opts.reset_nuke_untracked = 1;
> > +     else
> > +             opts.reset_keep_untracked = 1;
>
> In both cases opts.reset_keep_untracked is set to 1, I assume it's a
> mistake

No, it only sets opts.reset_keep_untracked to 1 when o->force is
false.  If o->force is true, it instead sets opts.reset_nuke_untracked
to 1.  There's no mistake there.

> >       opts.merge = 1;
> >       opts.fn = oneway_merge;
> >       opts.verbose_update = o->show_progress;
> >       opts.src_index = &the_index;
> >       opts.dst_index = &the_index;
> > +     if (o->overwrite_ignore) {
> > +             opts.dir = xcalloc(1, sizeof(*opts.dir));
>
> ditto potential leak.
>
> > +             opts.dir = xcalloc(1, sizeof(*opts.dir));
> > +             opts.dir->flags |= DIR_SHOW_IGNORED;
> > +             setup_standard_excludes(opts.dir);
> > +     }
>
>
> ditto (also more omitted).

Yep.