On Mon, Aug 12, 2024 at 2:39 PM Josh Steadmon <steadmon@xxxxxxxxxx> wrote:
>
> On 2024.08.12 10:10, 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.
> >
> > /*
> > * Namespace data types as well as functions and ensure consistent
> > * naming of data types and the functions that operate on them.
> > */
> > struct libgit_configset {
> > struct config_set set;
> > };
> >
> > struct libgit_configset *libgit_configset_new() {
> > struct libgit_configset *set = xmalloc(sizeof(*set));
> >
> > git_configset_init(&set->set);
> > return set;
> > }
> >
> > void libgit_configset_free(struct libgit_configset *set) {
> > git_configset_clear(&set->set);
> > free(set);
> > }
> >
> > Best Wishes
> >
> > Phillip
>
> Hmmm I see your point, but I am also hoping to keep the symbol export
> shim as small as possible, so that we can try to autogenerate it rather
> than add entries by hand. However, if people feel strongly that we don't
> want to add helper functions like *_alloc() or *_free() for types that don't
> already have them upstream, perhaps we can just put them in a separate
> rust-helpers.c file or something.
I'm thinking of this patch series as two closely related but
technically separable things: the creation of a .a/.so that can be
used outside of git, and the rust wrapper around that library. I think
these functions would be needed by all users of the library,
regardless of what language they're implemented in. i.e. they
shouldn't be thought of as 'rust helpers' and instead just the way
that the library is designed. _All_ functions that allocate memory
should have a paired "free" method, and that should be used
exclusively, regardless of host language.
So nit: I wouldn't call it rust-helpers.c ;)
