Re: [PATCH v2 4/5] config: add git_configset_alloc() and git_configset_clear_and_free()
On 2024.08.16 13:24, Patrick Steinhardt wrote:
Show 23 quoted lines
> On Mon, Aug 12, 2024 at 10:10:48AM +0100, Phillip Wood wrote:
> > Hi Josh
> >
> > On 09/08/2024 23:41, Josh Steadmon wrote:
> > > 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.
> >
> > Do we really need to add this code to config.c rather than handling it in
> > the wrapper layer in the next patch?
> >
> > Looking ahead I wonder how useful it is to users of the library to separate
> > out allocation from initialization. A function that allocates and
> > initializes a configset would be more convenient and harder to misuse.
> > Calling release functions *_free() rather than *_clear_and_free() would be
> > more convenient as well. I also noticed that the data types are not
> > namespaced when they are exported. So perhaps we could drop this patch and
> > add the following to the next patch.
>
> Agreed. This is also part of our coding guidelines as of 10f0723c8d
> (Documentation: document idiomatic function names, 2024-07-30).
>
> Patrick