Re: [PATCH v3 4/6] config: add git_configset_alloc() and git_configset_clear_and_free()
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 10, 2024, 06:41 UTC
- Message-ID
- <Zt_qM8ZGN4fnqUoY@pks.im>
- In-Reply-To
- <CAPig+cRKbvWT3Bx65-ULAYRSrrKTCL0C7r6xpNSdN5JjLBPjCQ@mail.gmail.com>
On Fri, Sep 06, 2024 at 07:24:22PM -0400, Eric Sunshine wrote:
Show 24 quoted lines
> On Fri, Sep 6, 2024 at 6:21 PM Calvin Wan <calvinwan@google.com> wrote:
> > From: Josh Steadmon <steadmon@google.com>
> >
> > Add git_configset_alloc() and git_configset_clear_and_free() functions
> > so that callers can manage config_set structs on the heap. This also
> > allows non-C external consumers to treat config_sets as opaque structs.
> >
> > Co-authored-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
>
> Almost all these patches suffer from some sort of missing Josh
> trailer, so I'll stop mentioning it now.
>
> > diff --git a/config.h b/config.h
> > @@ -472,6 +472,11 @@ struct config_set {
> > +/**
> > + * Alloc a config_set
> > + */
> > +struct config_set *git_configset_alloc(void);
>
> Should this documentation string mention that git_configset_alloc()
> does _not_ initialize the configset? Alternatively, should this
> function also initialize it as a convenience (and mention so in the
> documentation)?I'd think so. I don't quite see the point in splitting this up into two separate function calls. Is there ever a case where one wants to allocate the configset, but not use it?
Patrick