diff mbox series

[2/4] reftable/merged: fix zero-sized allocation when there are no readers

Message ID 20241221-b4-pks-reftable-oom-fix-without-readers-v1-2-12db83a3267c@pks.im (mailing list archive)
State New
Headers show
Series reftable: fix out-of-memory errors on NonStop | expand

Commit Message

Patrick Steinhardt Dec. 21, 2024, 11:50 a.m. UTC
It was reported [1c that Git started to fail with an out-of-memory error
when initializing repositories with the reftable backend on NonStop
platforms. A bisect led to 802c0646ac (reftable/merged: handle
allocation failures in `merged_table_init_iter()`, 2024-10-02), which
changed how we allocate memory when initializing a merged table.

The root cause of this seems to be that NonStop returns a `NULL` pointer
when doing a zero-sized allocation. This would've already happened
before the above change, but we never noticed because we did not check
the result. Now that we do we notice and thus return an out-of-memory
error to the caller.

Fix the issue by skipping the allocation altogether in case there are no
readers.

[1]: <00ad01db5017$aa9ce340$ffd6a9c0$@nexbridge.com>

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/merged.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Kristoffer Haugsbakk Dec. 21, 2024, 12:40 p.m. UTC | #1
Hi

On Sat, Dec 21, 2024, at 12:50, Patrick Steinhardt wrote:
> It was reported [1c

A Neo user, I see.

s/c/]/

> [snip]
> the result. Now that we do we notice and thus return an out-of-memory
> error to the caller.

s/Now that we do we/Now we do notice/

Repeat of “we”.  And “that” can’t be be used since it has nothing to
connect to.
Patrick Steinhardt Dec. 21, 2024, 2:18 p.m. UTC | #2
On Sat, Dec 21, 2024 at 01:40:32PM +0100, Kristoffer Haugsbakk wrote:
> Hi
> 
> On Sat, Dec 21, 2024, at 12:50, Patrick Steinhardt wrote:
> > It was reported [1c
> 
> A Neo user, I see.

There are dozens of us!

> s/c/]/

Oh, I even saw that typo, but obviously forgot to fix it.

> > [snip]
> > the result. Now that we do we notice and thus return an out-of-memory
> > error to the caller.
> 
> s/Now that we do we/Now we do notice/
> 
> Repeat of “we”.  And “that” can’t be be used since it has nothing to
> connect to.

Indeed. Will wait a bit to hear back from Randall before sending another
version. Thanks!

Patrick
diff mbox series

Patch

diff --git a/reftable/merged.c b/reftable/merged.c
index bb0836e3443271f9c0d5ba5582c78694d437ddc2..e72b39e178d4dec6ddfca970b5af71b71431397a 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -240,14 +240,16 @@  int merged_table_init_iter(struct reftable_merged_table *mt,
 			   struct reftable_iterator *it,
 			   uint8_t typ)
 {
-	struct merged_subiter *subiters;
+	struct merged_subiter *subiters = NULL;
 	struct merged_iter *mi = NULL;
 	int ret;
 
-	REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len);
-	if (!subiters) {
-		ret = REFTABLE_OUT_OF_MEMORY_ERROR;
-		goto out;
+	if (mt->readers_len) {
+		REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len);
+		if (!subiters) {
+			ret = REFTABLE_OUT_OF_MEMORY_ERROR;
+			goto out;
+		}
 	}
 
 	for (size_t i = 0; i < mt->readers_len; i++) {