git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  Edward Thomson <ethomson@edwardthomson.com>,
	Josh Steadmon <steadmon@google.com>,
	Calvin Wan <calvinwan@google.com>,
	Kyle Lippincott <spectral@google.com>
Subject: Re: [PATCH v2 04/22] reftable/basics: handle allocation failures in `reftable_calloc()`
Date: Thu, 26 Sep 2024 09:13:23 -0700	[thread overview]
Message-ID: <xmqqzfnul7fg.fsf@gitster.g> (raw)
In-Reply-To: <ZvVPiIzzLTTb75b8@pks.im> (Patrick Steinhardt's message of "Thu, 26 Sep 2024 14:11:52 +0200")

Patrick Steinhardt <ps@pks.im> writes:

>> But it illustrates why open coding is not necessarily an excellent
>> idea in the longer term, doesn't it?  When unsigned_mult_overflows()
>> is updated to avoid such a false positive, how would we remember
>> that we need to update this copy we?
>
> I agree in general, but with the reftable library I'm stuck between a
> rock and a hard place. My goal is to make it fully reusable by other
> projects without first having to do surgery on their side. While having
> something like `st_mult()` is simple enough to port over, the biggest
> problem I have is that over time we sneak in more and more code from the
> Git codebase. The result is death by a thousand cuts.

> Now if we had a single header that exposes a small set of functions
> without _anything_ else it could work alright. But I rather doubt that
> we really want to have a standalone header for `st_mult()` that we can
> include. But without such a standalone header it is simply too easy to
> start building on top of Git features by accident.
>
> So I'm instead aiming to go a different way and fully cut the reftable
> code loose from Git. So even if we e.g. eventually want `struct strbuf`
> return errors on failure, it would only address one part of my problem.

The dependency to "strbuf" (just as an example) was added initially
fairly early.  Soon after 27f7ed2a (reftable: add LICENSE,
2021-10-07) added the reftable/ hierarchy, e303bf22 (reftable:
(de)serialization for the polymorphic record type., 2021-10-07).  I
somehow had an impression that reftable "library" started without
any Git dependency and then use of our helper functions seemed
through from the shim layer, but it was pretty much part of the
library from day one, it seems.

> A couple months ago I said that I'll try to write something like shims
> that wrap our types in reftable types such that other projects can
> provide implementations for such shims themselves. I tried to make that
> work, but the result was an unholy mess that really didn't make any
> sense whatsoever. Most of the features that I need from the Git codebase
> can be provided in a couple of lines of code (`struct strbuf` is roughly
> 50 lines for example), plus maybe a callback function that can be
> provided to wire things up on the user side (`register_tempfiles()` for
> example). So once I saw that those wrappers are harder to use and result
> in roughly the same lines of code I decided to scrap that approach and
> instead try to convert it fully.
>
> So yeah, overall we shouldn't open-code things like this. But I don't
> really see another way to do this for the reftable library.

But isn't all of the above what Libification ought to be about?  I
was hoping that the reftable polishing would not have to be done by
you alone, and you would recruit those who are into libification of
other parts of Git codebase to help cleaning up these fringe (from
the point of view of reftable) interfaces.

What do the libification folks feel about this (folks involved in
libgit-sys CC'ed; of course all others who are interested in the
libification topic are welcome to comment)?

Thanks.

  reply	other threads:[~2024-09-26 16:13 UTC|newest]

Thread overview: 151+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16 12:28 [PATCH 00/22] reftable: handle allocation errors Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 01/22] reftable/error: introduce out-of-memory error code Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 02/22] reftable/basics: merge "publicbasics" into "basics" Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 03/22] reftable: introduce `reftable_strdup()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 04/22] reftable/basics: handle allocation failures in `reftable_calloc()` Patrick Steinhardt
2024-09-21 19:37   ` Junio C Hamano
2024-09-24  5:48     ` Patrick Steinhardt
2024-09-24  6:02       ` Patrick Steinhardt
2024-09-24 16:39       ` Junio C Hamano
2024-09-16 12:28 ` [PATCH 05/22] reftable/basics: handle allocation failures in `parse_names()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 06/22] reftable/record: handle allocation failures on copy Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 07/22] reftable/record: handle allocation failures when decoding records Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 08/22] reftable/writer: handle allocation failures in `writer_index_hash()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 09/22] reftable/writer: handle allocation failures in `reftable_new_writer()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 10/22] reftable/merged: handle allocation failures in `merged_table_init_iter()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 11/22] reftable/reader: handle allocation failures for unindexed reader Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 12/22] reftable/reader: handle allocation failures in `reader_init_iter()` Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 13/22] reftable/stack: handle allocation failures on reload Patrick Steinhardt
2024-09-16 12:28 ` [PATCH 14/22] reftable/stack: handle allocation failures in `reftable_new_stack()` Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 15/22] reftable/stack: handle allocation failures in `stack_compact_range()` Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 16/22] reftable/stack: handle allocation failures in auto compaction Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 17/22] reftable/iter: handle allocation failures when creating indexed table iter Patrick Steinhardt
2024-09-22  6:26   ` Junio C Hamano
2024-09-24  5:49     ` Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 18/22] reftable/blocksource: handle allocation failures Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 19/22] reftable/block: " Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 20/22] reftable/pq: handle allocation failures when adding entries Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 21/22] reftable/tree: handle allocation failures Patrick Steinhardt
2024-09-16 12:29 ` [PATCH 22/22] reftable: handle trivial " Patrick Steinhardt
2024-09-24  6:31 ` [PATCH v2 00/22] reftable: handle allocation errors Patrick Steinhardt
2024-09-24  6:31   ` [PATCH v2 01/22] reftable/error: introduce out-of-memory error code Patrick Steinhardt
2024-09-24  6:31   ` [PATCH v2 02/22] reftable/basics: merge "publicbasics" into "basics" Patrick Steinhardt
2024-09-24  6:31   ` [PATCH v2 03/22] reftable: introduce `reftable_strdup()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 04/22] reftable/basics: handle allocation failures in `reftable_calloc()` Patrick Steinhardt
2024-09-24 16:59     ` Junio C Hamano
2024-09-26 12:11       ` Patrick Steinhardt
2024-09-26 16:13         ` Junio C Hamano [this message]
2024-09-27  5:28           ` Patrick Steinhardt
2024-09-27 12:21             ` Han-Wen Nienhuys
2024-09-27 15:21               ` Junio C Hamano
2024-09-24  6:32   ` [PATCH v2 05/22] reftable/basics: handle allocation failures in `parse_names()` Patrick Steinhardt
2024-09-24 22:19     ` René Scharfe
2024-09-26 12:09       ` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 06/22] reftable/record: handle allocation failures on copy Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 07/22] reftable/record: handle allocation failures when decoding records Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 08/22] reftable/writer: handle allocation failures in `writer_index_hash()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 09/22] reftable/writer: handle allocation failures in `reftable_new_writer()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 10/22] reftable/merged: handle allocation failures in `merged_table_init_iter()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 11/22] reftable/reader: handle allocation failures for unindexed reader Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 12/22] reftable/reader: handle allocation failures in `reader_init_iter()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 13/22] reftable/stack: handle allocation failures on reload Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 14/22] reftable/stack: handle allocation failures in `reftable_new_stack()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 15/22] reftable/stack: handle allocation failures in `stack_compact_range()` Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 16/22] reftable/stack: handle allocation failures in auto compaction Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 17/22] reftable/iter: handle allocation failures when creating indexed table iter Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 18/22] reftable/blocksource: handle allocation failures Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 19/22] reftable/block: " Patrick Steinhardt
2024-09-24  6:32   ` [PATCH v2 20/22] reftable/pq: handle allocation failures when adding entries Patrick Steinhardt
2024-09-24  6:33   ` [PATCH v2 21/22] reftable/tree: handle allocation failures Patrick Steinhardt
2024-09-24  6:33   ` [PATCH v2 22/22] reftable: handle trivial " Patrick Steinhardt
2024-09-30  8:08 ` [PATCH v3 00/22] refatble: handle allocation errors Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 01/22] reftable/error: introduce out-of-memory error code Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 02/22] reftable/basics: merge "publicbasics" into "basics" Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 03/22] reftable: introduce `reftable_strdup()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 04/22] reftable/basics: handle allocation failures in `reftable_calloc()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 05/22] reftable/basics: handle allocation failures in `parse_names()` Patrick Steinhardt
2024-09-30 17:40     ` René Scharfe
2024-09-30  8:08   ` [PATCH v3 06/22] reftable/record: handle allocation failures on copy Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 07/22] reftable/record: handle allocation failures when decoding records Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 08/22] reftable/writer: handle allocation failures in `writer_index_hash()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 09/22] reftable/writer: handle allocation failures in `reftable_new_writer()` Patrick Steinhardt
2024-09-30 17:40     ` René Scharfe
2024-09-30 18:22       ` Patrick Steinhardt
2024-09-30 19:11         ` Junio C Hamano
2024-09-30  8:08   ` [PATCH v3 10/22] reftable/merged: handle allocation failures in `merged_table_init_iter()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 11/22] reftable/reader: handle allocation failures for unindexed reader Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 12/22] reftable/reader: handle allocation failures in `reader_init_iter()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 13/22] reftable/stack: handle allocation failures on reload Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 14/22] reftable/stack: handle allocation failures in `reftable_new_stack()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 15/22] reftable/stack: handle allocation failures in `stack_compact_range()` Patrick Steinhardt
2024-09-30  8:08   ` [PATCH v3 16/22] reftable/stack: handle allocation failures in auto compaction Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 17/22] reftable/iter: handle allocation failures when creating indexed table iter Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 18/22] reftable/blocksource: handle allocation failures Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 19/22] reftable/block: " Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 20/22] reftable/pq: handle allocation failures when adding entries Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 21/22] reftable/tree: handle allocation failures Patrick Steinhardt
2024-09-30  8:09   ` [PATCH v3 22/22] reftable: handle trivial " Patrick Steinhardt
2024-09-30 18:18   ` [PATCH v3 00/22] refatble: handle allocation errors Junio C Hamano
2024-10-01  9:41 ` [PATCH v4 00/25] reftable: " Patrick Steinhardt
2024-10-01  9:41   ` [PATCH v4 01/25] reftable/error: introduce out-of-memory error code Patrick Steinhardt
2024-10-01  9:41   ` [PATCH v4 02/25] reftable/basics: merge "publicbasics" into "basics" Patrick Steinhardt
2024-10-01  9:41   ` [PATCH v4 03/25] reftable: introduce `reftable_strdup()` Patrick Steinhardt
2024-10-01  9:41   ` [PATCH v4 04/25] reftable/basics: handle allocation failures in `reftable_calloc()` Patrick Steinhardt
2024-10-01  9:41   ` [PATCH v4 05/25] reftable/basics: handle allocation failures in `parse_names()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 06/25] reftable/record: handle allocation failures on copy Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 07/25] reftable/record: handle allocation failures when decoding records Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 08/25] reftable/writer: handle allocation failures in `writer_index_hash()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 09/25] reftable/writer: handle allocation failures in `reftable_new_writer()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 10/25] reftable/merged: handle allocation failures in `merged_table_init_iter()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 11/25] reftable/reader: handle allocation failures for unindexed reader Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 12/25] reftable/reader: handle allocation failures in `reader_init_iter()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 13/25] reftable/stack: handle allocation failures on reload Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 14/25] reftable/stack: handle allocation failures in `reftable_new_stack()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 15/25] reftable/stack: handle allocation failures in `stack_compact_range()` Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 16/25] reftable/stack: handle allocation failures in auto compaction Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 17/25] reftable/iter: handle allocation failures when creating indexed table iter Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 18/25] reftable/blocksource: handle allocation failures Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 19/25] reftable/block: " Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 20/25] reftable/pq: handle allocation failures when adding entries Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 21/25] reftable/tree: handle allocation failures Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 22/25] reftable: handle trivial " Patrick Steinhardt
2024-10-01  9:42   ` [PATCH v4 23/25] reftable: fix calls to free(3P) Patrick Steinhardt
2024-10-01  9:43   ` [PATCH v4 24/25] reftable: introduce `REFTABLE_FREE_AND_NULL()` Patrick Steinhardt
2024-10-01  9:43   ` [PATCH v4 25/25] reftable/basics: ban standard allocator functions Patrick Steinhardt
2024-10-01 22:50     ` Junio C Hamano
2024-10-02  4:30       ` Patrick Steinhardt
2024-10-01 17:52   ` [PATCH v4 00/25] reftable: handle allocation errors Junio C Hamano
2024-10-01 18:30     ` René Scharfe
2024-10-01 19:25       ` Junio C Hamano
2024-10-02  4:29         ` Patrick Steinhardt
2024-10-02 18:04           ` Junio C Hamano
2024-10-02 10:55 ` [PATCH v5 " Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 01/25] reftable/error: introduce out-of-memory error code Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 02/25] reftable/basics: merge "publicbasics" into "basics" Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 03/25] reftable: introduce `reftable_strdup()` Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 04/25] reftable/basics: handle allocation failures in `reftable_calloc()` Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 05/25] reftable/basics: handle allocation failures in `parse_names()` Patrick Steinhardt
2024-10-02 22:07     ` Eric Sunshine
2024-10-04  4:58       ` Patrick Steinhardt
2024-10-04  5:43         ` Eric Sunshine
2024-10-02 10:55   ` [PATCH v5 06/25] reftable/record: handle allocation failures on copy Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 07/25] reftable/record: handle allocation failures when decoding records Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 08/25] reftable/writer: handle allocation failures in `writer_index_hash()` Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 09/25] reftable/writer: handle allocation failures in `reftable_new_writer()` Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 10/25] reftable/merged: handle allocation failures in `merged_table_init_iter()` Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 11/25] reftable/reader: handle allocation failures for unindexed reader Patrick Steinhardt
2024-10-02 10:55   ` [PATCH v5 12/25] reftable/reader: handle allocation failures in `reader_init_iter()` Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 13/25] reftable/stack: handle allocation failures on reload Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 14/25] reftable/stack: handle allocation failures in `reftable_new_stack()` Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 15/25] reftable/stack: handle allocation failures in `stack_compact_range()` Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 16/25] reftable/stack: handle allocation failures in auto compaction Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 17/25] reftable/iter: handle allocation failures when creating indexed table iter Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 18/25] reftable/blocksource: handle allocation failures Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 19/25] reftable/block: " Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 20/25] reftable/pq: handle allocation failures when adding entries Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 21/25] reftable/tree: handle allocation failures Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 22/25] reftable: handle trivial " Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 23/25] reftable: fix calls to free(3P) Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 24/25] reftable: introduce `REFTABLE_FREE_AND_NULL()` Patrick Steinhardt
2024-10-02 10:56   ` [PATCH v5 25/25] reftable/basics: ban standard allocator functions Patrick Steinhardt
2024-10-02 19:32   ` [PATCH v5 00/25] reftable: handle allocation errors 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=xmqqzfnul7fg.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=calvinwan@google.com \
    --cc=ethomson@edwardthomson.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --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 NNTP newsgroup(s).