All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Randall S. Becker" <randall.becker@nexbridge.ca>
Subject: Re: [PATCH 3/4] reftable/stack: fix zero-sized allocation when there are no readers
Date: Sun, 22 Dec 2024 08:13:39 +0100	[thread overview]
Message-ID: <Z2e8I6eGcTXL7uec@pks.im> (raw)
In-Reply-To: <xmqq34ihc4zt.fsf@gitster.g>

On Sat, Dec 21, 2024 at 09:36:54AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Similar as the preceding commit, we may try to do a zero-sized
> > allocation when reloading a reftable stack that ain't got any tables.
> > It is implementation-defined whether malloc(3p) returns a NULL pointer
> > in that case or a zero-sized object. In case it does return a NULL
> > pointer though it causes us to think we have run into an out-of-memory
> > situation, and thus we return an error.
> >
> > Fix this by only allocating arrays when they have at least one entry.
> > Refactor the code so that we don't try to access those arrays in case
> > they are empty.
> >
> > Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  reftable/stack.c | 44 +++++++++++++++++++++++---------------------
> >  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> This somehow did not cleanly apply, so I whiggled it in manually.
> 
> I probably wouldn't mixed the "size_t i" changes into this fix if I
> were doing it.  To avoid "while (*names)" loop, I would have made it
> to "for (size_t j = 0; j < names_len; j++)" and kept the existing
> use of "i" intact, instead.  And reintroducing for() scoped "i"
> three times did not seem to make it easier to read the result.
> 
> I am not convinced we need to avoid "while (*names)", by the way.
> If "names" were NULL, names_length() would already have segfaulted
> anyway, and basics.c:parse_names(), when not returning NULL (which
> would have been caught by the sole caller of reload_once() as an
> error), makes sure it gives its caller a NULL-terminated array.

True. I was initially erring on the side of being overly cautious here
because I didn't yet have the idea to adapt `reftable_malloc()` and
`reftable_realloc()` so that they have the same behaviour as NonStop.
Let me redo this change and reduce it to the minimum required one.

Patrick

  parent reply	other threads:[~2024-12-22  7:13 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-21 11:50 [PATCH 0/4] reftable: fix out-of-memory errors on NonStop Patrick Steinhardt
2024-12-21 11:50 ` [PATCH 1/4] reftable/stack: don't perform auto-compaction with less than two tables Patrick Steinhardt
2024-12-21 17:08   ` Junio C Hamano
2024-12-21 17:51     ` Junio C Hamano
2024-12-22  7:13       ` Patrick Steinhardt
2024-12-21 11:50 ` [PATCH 2/4] reftable/merged: fix zero-sized allocation when there are no readers Patrick Steinhardt
2024-12-21 12:40   ` Kristoffer Haugsbakk
2024-12-21 14:18     ` Patrick Steinhardt
2024-12-21 17:13     ` Junio C Hamano
2024-12-21 11:50 ` [PATCH 3/4] reftable/stack: " Patrick Steinhardt
2024-12-21 17:36   ` Junio C Hamano
2024-12-21 17:57     ` Junio C Hamano
2024-12-21 18:06       ` rsbecker
2024-12-21 18:30         ` Junio C Hamano
2024-12-22 17:48           ` rsbecker
2024-12-22 18:17           ` rsbecker
2024-12-22 18:35             ` Patrick Steinhardt
2024-12-23  4:08               ` Junio C Hamano
2024-12-22  7:13     ` Patrick Steinhardt [this message]
2024-12-21 11:50 ` [PATCH 4/4] reftable/basics: return NULL on zero-sized allocations Patrick Steinhardt
2024-12-21 12:53   ` Kristoffer Haugsbakk
2024-12-21 15:05 ` [PATCH 0/4] reftable: fix out-of-memory errors on NonStop Randall Becker
2024-12-21 17:43   ` Junio C Hamano
2024-12-21 16:52 ` Junio C Hamano
2024-12-22  7:24 ` [PATCH v2 " Patrick Steinhardt
2024-12-22  7:24   ` [PATCH v2 1/4] reftable/stack: don't perform auto-compaction with less than two tables Patrick Steinhardt
2024-12-22  7:24   ` [PATCH v2 2/4] reftable/merged: fix zero-sized allocation when there are no readers Patrick Steinhardt
2024-12-22  7:24   ` [PATCH v2 3/4] reftable/stack: " Patrick Steinhardt
2024-12-22  7:24   ` [PATCH v2 4/4] reftable/basics: return NULL on zero-sized allocations Patrick Steinhardt
2024-12-22 16:31   ` [PATCH v2 0/4] reftable: fix out-of-memory errors on NonStop 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=Z2e8I6eGcTXL7uec@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=randall.becker@nexbridge.ca \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.