Hi Calvin
On 22/04/2024 17:26, Calvin Wan wrote:
The first idea involves turning `strbuf_grow()` into a wrapper function
that invokes its equivalent library function, eg.
`libgit_strbuf_grow()`:
int libgit_strbuf_grow(struct strbuf *sb, size_t extra)
{
int new_buf = !sb->alloc;
if (unsigned_add_overflows(extra, 1) ||
unsigned_add_overflows(sb->len, extra + 1))
return -1;
if (new_buf)
sb->buf = NULL;
ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
if (new_buf)
sb->buf[0] = '\0';
return 0;
}
void strbuf_grow(struct strbuf *sb, size_t extra)
{
if (libgit_strbuf_grow(sb, extra))
die("you want to use way too much memory");
}
(Note a context object could also be added as a parameter to
`libgit_strbuf_grow()` for error messages and possibly global variables.)
I agree with Junio that this is a good route forward and that we should
not consider removing the wrappers to be part of the libification
project. ALLOC_GROW() can die so I think we'd need a way to propagate an
error message up to the wrapper even for relatively simple functions
like this. For the allocation functions we'd either need to use a static
string for the message which is not a good fit for other functions that
want to dynamically format their messages (for example to include a path
name into the error message), or pre-allocate the error messages.
The shortfall of this approach is that we'd be carrying two different
functions for every library function until we are able to remove all of
them. It would also create additional toil for Git contributors to
figure out which version of the function should be used.
Hopefully it should be clear if a function is part of the library or not
so I don't think this should be a big problem.
Best Wishes
Phillip
