From: phillip.wood123@gmail.com
To: Calvin Wan <calvinwan@google.com>
Cc: Git Mailing List <git@vger.kernel.org>,
"brian m. carlson" <sandals@crustytoothpaste.net>,
rsbecker@nexbridge.com, phillip.wood@dunelm.org.uk,
Kyle Lippincott <spectral@google.com>,
Josh Steadmon <steadmon@google.com>,
Emily Shaffer <nasamuffin@google.com>,
Enrico Mrass <emrass@google.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [RFD] Libification proposal: separate internal and external interfaces
Date: Tue, 23 Apr 2024 10:57:31 +0100 [thread overview]
Message-ID: <cb2ffdb1-5e6c-4dd3-89d4-f0b31f2f17ec@gmail.com> (raw)
In-Reply-To: <20240422162617.308366-1-calvinwan@google.com>
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
next prev parent reply other threads:[~2024-04-23 9:57 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-02 14:18 [RFD] Libification proposal: separate internal and external interfaces Calvin Wan
2024-04-07 21:33 ` brian m. carlson
2024-04-07 21:48 ` rsbecker
2024-04-08 1:09 ` brian m. carlson
2024-04-08 11:07 ` rsbecker
2024-04-08 21:29 ` Junio C Hamano
2024-04-09 0:35 ` brian m. carlson
2024-04-09 17:26 ` Calvin Wan
2024-04-09 9:40 ` Phillip Wood
2024-04-09 17:30 ` Calvin Wan
2024-04-22 16:26 ` Calvin Wan
2024-04-22 20:28 ` Junio C Hamano
2024-04-23 9:57 ` phillip.wood123 [this message]
2024-05-09 1:00 ` Kyle Lippincott
2024-05-10 9:52 ` Phillip Wood
2024-05-10 21:35 ` Kyle Lippincott
2024-05-09 19:45 ` Kyle Lippincott
2024-05-09 20:14 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cb2ffdb1-5e6c-4dd3-89d4-f0b31f2f17ec@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=calvinwan@google.com \
--cc=emrass@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=nasamuffin@google.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=rsbecker@nexbridge.com \
--cc=sandals@crustytoothpaste.net \
--cc=spectral@google.com \
--cc=steadmon@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).