On Mon, Dec 03, 2018 at 11:10:49AM -0800, Matthew DeVore wrote:
> > > + memset(&s_r_opt, 0, sizeof(s_r_opt));
> > > + s_r_opt.allow_exclude_promisor_objects = 1;
> > > + setup_revisions(ac, av, &revs, &s_r_opt);
> >
> > I wonder if a static initializer for setup_revision_opt is worth it. It
> > would remove the need for this memset. Probably not a big deal either
> > way, though.
> I think you mean something like this:
>
> static struct setup_revision_opt s_r_opt = {NULL, NULL, NULL, 0, 1, 0};
>
> This is a bit cryptic (I have to read the struct declaration in order to
> know what is being set to 1) and if the struct ever gets a new field before
> allow_exclude_promisor_objects, this initializer has to be updated.
I agree that's pretty awful. I meant something like this:
struct setup_revision_opt s_r_opt = { NULL };
...
s_r_opt.allow_exclude_promisor_objects = 1;
setup_revisions(...);
It's functionally equivalent to the memset(), but you don't have to
wonder about whether we peek at the uninitialized state in between.
That said, our C99 designated initializer weather-balloons haven't
gotten any complaints yet. So I think you could actually do:
struct setup_revision_opt s_r_opt = {
.allow_exclude_promisor_objects = 1,
};
...
setup_revisions(...);
which is pretty nice.
-Peff
