On Wed, Sep 29, 2021 at 11:32 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> On Wed, Sep 29 2021, Elijah Newren wrote:
>
> > On Wed, Sep 29, 2021 at 2:27 AM Ævar Arnfjörð Bjarmason
> > <avarab@xxxxxxxxx> wrote:
> >>
...
> >>
> >> I think getting rid of the boilerplate makes sense, but it doesn't sound
> >> from the commit message like you've considered just making that "struct
> >> dir*" member a "struct dir" instead.
> >>
> >> That simplifies things a lot, i.e. we can just DIR_INIT it, and don't
> >> need every caller to malloc/free it.
> >
> > See the next patch in the series. :-)
>
> Ah!
>
> >> Sometimes a pointer makes sense, but in this case the "struct
> >> unpack_trees_options" can just own it.
> >
> > I did make it internal to unpack_trees_options in the next patch, but
> > kept it as a pointer just because that let me know whether it was used
> > or not. I guess I could have added a boolean as well. But I don't
> > actually allocate anything, because it's either a NULL pointer, or a
> > pointer to something on the stack. So, I do get to just use DIR_INIT.
>
> I think I'm probably missing something. I just made it allocated on the
> stack by the caller using "struct unpack_trees_options", but then you
> end up having a dir* in the struct, but that's only filled in as a
> pointer to the stack variable? Maybe there's some subtlety I'm missing
> here...
As per the next patch:
int unpack_trees(..., struct unpack_trees_options *o)
{
struct dir_struct dir = DIR_INIT;
...
if (!o->preserve_ignored) {
/* Setup 'dir', make o->dir point to it */
....
o->dir = &dir;
}
...
if (o->dir)
/* cleanup */
....
}
The caller doesn't touch o->dir (other than initializing it to zeros);
unpack_trees() is wholly responsible for it. I'd kind of like to
entirely remove dir from unpack_trees_options(), but I need a way of
passing it down through all the other functions in unpack-trees.c, and
leaving it in unpack_trees_options seems the easiest way to do so. So
I just marked it as "for internal use only".
