On 2024.08.08 13:18, Kyle Lippincott wrote:

On Thu, Aug 8, 2024 at 11:22 AM Josh Steadmon [off-list ref] wrote:

quoted

On 2024.08.07 23:55, brian m. carlson wrote:

quoted

On 2024-08-07 at 23:05:00, Josh Steadmon wrote:

quoted

Yeah, needing to free() is the only thing we striclty need from libc
right now. Please correct me if I'm wrong, but IIUC then any memory that
is allocated on the C side and then passed to Rust needs one of:
1) freed by libc::free() on the Rust side,
2) passed back to the C side to be freed there, or
3) leaked

Am I correct in assuming that your opinion is that writing additional
*_free() functions on the C side is worth it to avoid libc? If so, then
I'm fine with including that in V2.

I think if we're going to be writing a general purpose API for
libification, we probably should provide free functions.  Normally, that
will be a call to free(3)

[snip]

So in this case, does that mean we'd replace our call to `libc::free()`
with just `free()`, and then add a declaration for `free` in our
`extern "C"` section of cgit-sys? It seems to work on my machine, but is
that actually the more portable option compared to using libc::free? Or
have I misunderstood something?

I think both having a generic 'free' function, or requiring your API
consumer to have a compatible 'free' function is undesirable. If the
API hands you something that you must return/free, there should be a
function for that specifically. So I would expect if the API has a
`libgit_foo_get(foo** f)` function, there'd be a paired
`libgit_foo_release(foo* f)` (ignoring whatever squabbles we want to
have about the names). Requiring `libgit_foo_get(foo** f)` to be
paired with `libc::free(f)` limits us to always using libc malloc;
pairing it with `libgit_free((void*)f)` means we can't refcount it /
ignore it if it's a part of a parent object, etc.

Sorry, the diff context got removed, the the particular case I was
talking about here is where we get back a strdup()ed string from some of
the config API calls. I guess we could add a c_str_free() somewhere on
the Git side and call that, but it seems like a bit of overkill.

I do agree with you about providing _free() [or in our case,
_clear_and_free()] functions for more complicated data types though.