From patchwork Mon Dec 11 09:07:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486857 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="RVqA1L5e"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="pNf6SHDY" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A2B0AC3 for ; Mon, 11 Dec 2023 01:07:34 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 997F13202629; Mon, 11 Dec 2023 04:07:33 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 11 Dec 2023 04:07:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285653; x=1702372053; bh=MP Px12L6ZY4rNKw5Pnejundaqx57AtRza1Skh5nhY9M=; b=RVqA1L5epVoMOueHKq womMqcK4TjE/vDjlmQ+9duB3b8Zf97zzEKx5nQ5mqZI7wWkoBlVlrKdm7wMVaDtA BnKyTt9hz5cPJzPGKuvGlVGim7WIZfcplIU8/PyiI7ByMzJwNmjFEl5D8M5fXkji P7GjIgOLrXrYOkdwTq/8bMO3gP2WV1lkbEZEehPPEYY69Hhb5oHCQ+nzZ355Y/Ce AiagE/+gTjOuJ2YLxAlz9IYfiFElBSOOZctJqVM0W/pLyL9VgmEMWaCWy9AlG1uv eHe0Nk/Wp+ZXcGVYVo6s2wjGw5+d6UhMHTxEQfCKhc7IbRLD2/tT83H1FPTb8FqA mCeQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285653; x=1702372053; bh=MPPx12L6ZY4rN Kw5Pnejundaqx57AtRza1Skh5nhY9M=; b=pNf6SHDYtO4DytvT3j8oz9Z9Z2DB9 izYAGDI8KVxsvR7S71ERnyFGlJS7lGgtkbcCAZGkDdUcItvB2oltcUX6HSBu664e z9qkvCEvuukH4auziN73UNC/JIvr/+SM32qvVZAWWeRbBCwGOt+De9ZZbM0h3P9K LV7AdMH+I757VykvGZSnkWkYgghnaoHldeb+nu3wgWwiotyR+8NlYSlvl7vmhwKr PXkm+MBeMdR7tQrKHB3dvJjQC2aR0eLqKYWrZ69OH9qkdzoI6v1p/8CAEe3cspA9 M0/4H5kab7UFISXz1tENb3twQ0CmZ2/5gWBO+oYOyk9PcZMz+px+pjpcQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:31 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e701bf95 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:05:55 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:29 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 01/11] reftable: wrap EXPECT macros in do/while Message-ID: <5b2a64ca9fa1d290b5b7838ad15c0894b65ae777.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The `EXPECT` macros used by the reftable test framework are all using a single `if` statement with the actual condition. This results in weird syntax when using them in if/else statements like the following: ``` if (foo) EXPECT(foo == 2) else EXPECT(bar == 2) ``` Note that there need not be a trailing semicolon. Furthermore, it is not immediately obvious whether the else now belongs to the `if (foo)` or whether it belongs to the expanded `if (foo == 2)` from the macro. Fix this by wrapping the macros in a do/while loop. Signed-off-by: Patrick Steinhardt --- reftable/test_framework.h | 58 +++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/reftable/test_framework.h b/reftable/test_framework.h index 774cb275bf..ee44f735ae 100644 --- a/reftable/test_framework.h +++ b/reftable/test_framework.h @@ -12,32 +12,38 @@ license that can be found in the LICENSE file or at #include "system.h" #include "reftable-error.h" -#define EXPECT_ERR(c) \ - if (c != 0) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \ - __FILE__, __LINE__, c, reftable_error_str(c)); \ - abort(); \ - } - -#define EXPECT_STREQ(a, b) \ - if (strcmp(a, b)) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \ - __LINE__, #a, a, #b, b); \ - abort(); \ - } - -#define EXPECT(c) \ - if (!(c)) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \ - __LINE__, #c); \ - abort(); \ - } +#define EXPECT_ERR(c) \ + do { \ + if (c != 0) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \ + __FILE__, __LINE__, c, reftable_error_str(c)); \ + abort(); \ + } \ + } while (0) + +#define EXPECT_STREQ(a, b) \ + do { \ + if (strcmp(a, b)) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \ + __LINE__, #a, a, #b, b); \ + abort(); \ + } \ + } while (0) + +#define EXPECT(c) \ + do { \ + if (!(c)) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \ + __LINE__, #c); \ + abort(); \ + } \ + } while (0) #define RUN_TEST(f) \ fprintf(stderr, "running %s\n", #f); \ From patchwork Mon Dec 11 09:07:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486859 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Go7MMFve"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="bEDBbi0t" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 08014DF for ; Mon, 11 Dec 2023 01:07:38 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 2288F320261E; Mon, 11 Dec 2023 04:07:37 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 11 Dec 2023 04:07:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285656; x=1702372056; bh=3K FSWMNUw2QtyMjQHIzOfM74ETwtxfNqimrqZipOlhU=; b=Go7MMFveDvoHzPI+DE zw8oT5l5+HEA5xxeWGjDIy2PHRDI2P6Z03JpNjVNeU5I/ndC+EHioWyjsPESi+a3 JEQBaqdTKiCD4IYx5huq2gcexw18nmS/WjfQU+tUgAOp75jLSyw+GAWMM4yjP6oJ kSLLI0tnSNJdsNkS5y0Ut4PwOo+/2eA407sQUBuCZscBYWSruEp4aDjiPKvy4q0I JvdEydi1uqGANL0A4wwdyxPv2kDF6DkwRoOv6mZslH2vqroNOH4jlosZQ/hIB++k qO/Db60Px95ZJYeVm1bcSJen+e5fsKYF958gmT0R/wbp+GviJ5FWuB3LuQtLaNk7 W91g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285656; x=1702372056; bh=3KFSWMNUw2Qty MjQHIzOfM74ETwtxfNqimrqZipOlhU=; b=bEDBbi0tNjkw6g2gsahRKquJSbOyt HlwFsTsz5TJIqbJsIOE+fIBub0fQZ4xlIYvLWe6/GuJ5kmavY7MDDP68cCwoZK4y a3pgYCd95WLVPb8mu+AueS/RDPvyPWoG2yQTaAmGBXo4a9FgbDhXbIDdT0DDA1bd 3UJEYL9VzTU0jbic5ms0BeOgyFA/U9T9UHSe72dZ4SParTe13SdIWloXQk5eEb4O 1OAxSl0Xl/lLx1RSSeS9qpqE5FsTNrHA0bBpp8Q0CTuDLYy4UHRHeJd0p8T3RFlP LZc4jZ9cQrLNF3aMVZF1cdG7yasgULJ6jpkcuNfXjYlT4voRA29Gf5kOg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:35 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id b88dbf0d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:05:59 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:34 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 02/11] reftable: handle interrupted reads Message-ID: <3e8e63ece52c507296b012be6632dbed568ff97d.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: There are calls to pread(3P) and read(3P) where we don't properly handle interrupts. Convert them to use `pread_in_full()` and `read_in_full()`, respectively. Signed-off-by: Patrick Steinhardt --- reftable/blocksource.c | 2 +- reftable/stack.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/reftable/blocksource.c b/reftable/blocksource.c index 8331b34e82..a1ea304429 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -109,7 +109,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off, struct file_block_source *b = v; assert(off + size <= b->size); dest->data = reftable_malloc(size); - if (pread(b->fd, dest->data, size, off) != size) + if (pread_in_full(b->fd, dest->data, size, off) != size) return -1; dest->len = size; return size; diff --git a/reftable/stack.c b/reftable/stack.c index ddbdf1b9c8..ed108a929b 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -92,7 +92,7 @@ static int fd_read_lines(int fd, char ***namesp) } buf = reftable_malloc(size + 1); - if (read(fd, buf, size) != size) { + if (read_in_full(fd, buf, size) != size) { err = REFTABLE_IO_ERROR; goto done; } From patchwork Mon Dec 11 09:07:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486860 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="J++CUc1v"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="HKaHV/N4" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 82BCBD9 for ; Mon, 11 Dec 2023 01:07:42 -0800 (PST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 9C9B43202657; Mon, 11 Dec 2023 04:07:41 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Mon, 11 Dec 2023 04:07:42 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285661; x=1702372061; bh=HK rsSDIRCHkrbjb8BAIQRDSXxURBnxmliYt0D/tQEWM=; b=J++CUc1vN7w831H4sx +wvmwittll9BbXzamwl4RytjzB3wPYvr9f6YtVNzQ2cmnboXWHPkjIYQ9IjeMk0J MrO7b1tL0p7l3ctExgZHm1VDAMo0NRDmfrhSaNCD4VyqOD2DN5hztX/zyR0M0+nw cw0MRNpNLcZw/h67Kkl8wzPwwzjSTvXVaHW4lMRpn5lEXQziD3ljkWJDxlVCYY+y ihk/mP2LvOAnqYPTtrbpYbkjSlap1UjtWsuu3F2/ITFVmhrhLsQvyVFR6l/c5Ee0 /aLwe05k27HUohaw+rw6AVamXmm4wOhGZ/2+99I9fVZ+2IBNzEBxFp/WthxpWh6h OCeg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285661; x=1702372061; bh=HKrsSDIRCHkrb jb8BAIQRDSXxURBnxmliYt0D/tQEWM=; b=HKaHV/N4eEY9NN2AvzOHJsZeqpfqa lZM9DhTyXHwIG4TBVD7myzW1opefhg+2ndvSz83QpoEO5FhTLp8TvrjFsTr5ZuSR bFEw2NSCDHfflz91IMXHqaZGUvmmNa8Jk+ZvGyv2tUqSCdo4unAlDNjtIzCOy7/R ke9utb/egGMpwfWk5Otr6vaxI07UjDqsqcJwy6xsB7Vj41kN84Ep0r34ZlGpOqvc TF92OmEuTq+sPcagURTLUTYvXTs1AQLbNQmF0xrSC/xjLVsKFKKUagikRPTBN5yX 3rK5ECPSWCWZYirl2N6jIifR3PSN3WyVTGxBooMVGcp0SJV5Pmk4AYmvA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:39 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c189136f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:03 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:38 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 03/11] reftable: handle interrupted writes Message-ID: <1700d00d1ca017730d9188caf4eff9c02720131a.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: There are calls to write(3P) where we don't properly handle interrupts. Convert them to use `write_in_full()`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 6 +++--- reftable/stack_test.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index ed108a929b..f0cadad490 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -42,7 +42,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st, static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) { int *fdp = (int *)arg; - return write(*fdp, data, sz); + return write_in_full(*fdp, data, sz); } int reftable_new_stack(struct reftable_stack **dest, const char *dir, @@ -554,7 +554,7 @@ int reftable_addition_commit(struct reftable_addition *add) strbuf_addstr(&table_list, "\n"); } - err = write(add->lock_file_fd, table_list.buf, table_list.len); + err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len); strbuf_release(&table_list); if (err < 0) { err = REFTABLE_IO_ERROR; @@ -1024,7 +1024,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, strbuf_addstr(&ref_list_contents, "\n"); } - err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len); + err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index d0b717510f..0644c8ad2e 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -78,7 +78,7 @@ static void test_read_file(void) int i = 0; EXPECT(fd > 0); - n = write(fd, out, strlen(out)); + n = write_in_full(fd, out, strlen(out)); EXPECT(n == strlen(out)); err = close(fd); EXPECT(err >= 0); From patchwork Mon Dec 11 09:07:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486861 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="oEf3zfjk"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="qhZNrB92" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D1DA7D9 for ; Mon, 11 Dec 2023 01:07:46 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id E92BE3202677; Mon, 11 Dec 2023 04:07:45 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 11 Dec 2023 04:07:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285665; x=1702372065; bh=8g bLZFvksjaSZCwzhiJWV1hMVJc7TCifkkebfE7qm+w=; b=oEf3zfjk0wMuSDDM2l D/J4Hzd3E9Y1J7kUT4+6YbJIXvNRZ68Kh5eZtPR/AkMzGjz5zkn/jsmyTIoC0wNZ sP2bWhCcv17wlKCEGXPcuYZ1jYbLQnlYOSDuYyrQQ6v5SdK/0ZohR+2gZGsTKRdu 70SdL+JaLJYR+0/2aKXEjhbn/eso7geJ2Cnt2vLp3D/XndLsr8QJUTRFC7ko/GIf bZwwqgbfQz96LZmDfq6ebRj7eUR6ONFNH0yXqePp1oKJ8oHEZh0pzRRZVxH1CtmV vZdYpoxNnjUNI2kTUwTC9prAqD9UDfpuixkOjdZY70msh0I1pD5fXh1soTfuVYMg jEiA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285665; x=1702372065; bh=8gbLZFvksjaSZ CwzhiJWV1hMVJc7TCifkkebfE7qm+w=; b=qhZNrB92NMvJQawF2mb63GxYpjT9M /E3ifIbJik3MFSx+rKR8nz2LN/FWdc0TZtXe4bVpyukBtuZsTUC4YEoaVMocGcW+ dTEEOMUjoC42WQvvVohCQ85F0dG8S91sooEuhI91tEM2vBSoPV4q3yb3iUE12CLS jDM2o7JZJvPUgvMt1n61JWpT7FZDOx3k32QmD9SmjOe5IOS9GxvD39rnEZyt6C1H vMQntFpSPociOorayC8FjUyBO4nfYFtUgvmfxqKmjvlG9xkydUmbNyWqNfzSk57+ Kr59cQbbQBv73aKbtIgqtviq5aaqmzhSn8gsWZbduisByUzwgkRSqAG/w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:44 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 63b172e6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:07 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:42 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction Message-ID: <5e27d0a5566d90969734e92984cfafe6048924f4.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: While we have several tests that check whether we correctly perform auto-compaction when manually calling `reftable_stack_auto_compact()`, we don't have any tests that verify whether `reftable_stack_add()` does call it automatically. Add one. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 49 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 0644c8ad2e..52b4dc3b14 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -850,6 +850,54 @@ static void test_reftable_stack_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_add_performs_auto_compaction(void) +{ + struct reftable_write_options cfg = { 0 }; + struct reftable_stack *st = NULL; + struct strbuf refname = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err, i, n = 20; + + err = reftable_new_stack(&st, dir, cfg); + EXPECT_ERR(err); + + for (i = 0; i <= n; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_SYMREF, + .value.symref = "master", + }; + + /* + * Disable auto-compaction for all but the last runs. Like this + * we can ensure that we indeed honor this setting and have + * better control over when exactly auto compaction runs. + */ + st->disable_auto_compact = i != n; + + strbuf_reset(&refname); + strbuf_addf(&refname, "branch-%04d", i); + ref.refname = refname.buf; + + err = reftable_stack_add(st, &write_test_ref, &ref); + EXPECT_ERR(err); + + /* + * The stack length should grow continuously for all runs where + * auto compaction is disabled. When enabled, we should merge + * all tables in the stack. + */ + if (i != n) + EXPECT(st->merged->stack_len == i + 1); + else + EXPECT(st->merged->stack_len == 1); + } + + reftable_stack_destroy(st); + strbuf_release(&refname); + clear_dir(dir); +} + static void test_reftable_stack_compaction_concurrent(void) { struct reftable_write_options cfg = { 0 }; @@ -960,6 +1008,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_add); RUN_TEST(test_reftable_stack_add_one); RUN_TEST(test_reftable_stack_auto_compaction); + RUN_TEST(test_reftable_stack_add_performs_auto_compaction); RUN_TEST(test_reftable_stack_compaction_concurrent); RUN_TEST(test_reftable_stack_compaction_concurrent_clean); RUN_TEST(test_reftable_stack_hash_id); From patchwork Mon Dec 11 09:07:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486862 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="TgubDAcw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="aYDw7/5c" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4BF66D9 for ; Mon, 11 Dec 2023 01:07:51 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 66022320268D; Mon, 11 Dec 2023 04:07:50 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 11 Dec 2023 04:07:50 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285669; x=1702372069; bh=e8 uVX+p/aIJqUt9uBvRrCYzrQgoLhUP0CGh3m2N1vDQ=; b=TgubDAcwRGKcvAXzOE /FbE7ppsShUi8prU0B4Ji6iBYnRwEHvna7t92Pk/bnYA1FggP0Ju9ZkQ26MR2h3M 0tpDT+hhQX82JJNe6L2qha7fKv+GDERgDYUipAzVBapfVEO6kLUzNlqXktyzRpkK ELO09Dyt38eHuDbzp0+mTmE8MKhfc3Bp6zKA7W6kuyRnaUQzmpNiDNq7pjqyDU/K joZfpezRiQZxmiA55c9vaoPEIJx4/qieVhQFW9gYE0sLXywX+GeK11n9RUiBgdLz iluBWg3V87qYls9O2NEaeoDHqKRUvuaU5kP0WMm6buxEywN3SCBktOJVunv+ZdXS xBcw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285669; x=1702372069; bh=e8uVX+p/aIJqU t9uBvRrCYzrQgoLhUP0CGh3m2N1vDQ=; b=aYDw7/5c84889yEA7M+Eu+EY7nUZM jFRlqaEJ8Z6l3D5px4RlUIBmm6Y71UFeKvA/0khKlOeManUdrp4+Xee9Y9TzBAlL mRC0hBbXK7GadiqAV99CCm3uc635XglZELqH5pF2MDnPQnTnFQiqHbfMidTFLLql SIyEg5xnfBa7XiVp4mC09hojMjlcrHoTfkuL2G49wRnebB71xT963QtFzNHCyD2H VaIr1f+bR3xPr9NF0yZ9YlR89At71/oJ58R2N5t49NbQyr0hVdW41b1JgaovAeS9 MS5z9SVK+kLw4zIKMNj8Zo0UpekQiwyMsTV6/8l9TRsZAx8OIImhTDiuA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:48 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1d1e06ba (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:12 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:46 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 05/11] reftable/stack: perform auto-compaction with transactional interface Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Whenever updating references or reflog entries in the reftable stack, we need to add a new table to the stack, thus growing the stack's length by one. The stack can grow to become quite long rather quickly, leading to performance issues when trying to read records. But besides performance issues, this can also lead to exhaustion of file descriptors very rapidly as every single table requires a separate descriptor when opening the stack. While git-pack-refs(1) fixes this issue for us by merging the tables, it runs too irregularly to keep the length of the stack within reasonable limits. This is why the reftable stack has an auto-compaction mechanism: `reftable_stack_add()` will call `reftable_stack_auto_compact()` after its added the new table, which will auto-compact the stack as required. But while this logic works alright for `reftable_stack_add()`, we do not do the same in `reftable_addition_commit()`, which is the transactional equivalent to the former function that allows us to write multiple updates to the stack atomically. Consequentially, we will easily run into file descriptor exhaustion in code paths that use many separate transactions like e.g. non-atomic fetches. Fix this issue by calling `reftable_stack_auto_compact()`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 6 +++++ reftable/stack_test.c | 56 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/reftable/stack.c b/reftable/stack.c index f0cadad490..f5d18a842a 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -584,6 +584,12 @@ int reftable_addition_commit(struct reftable_addition *add) add->new_tables_len = 0; err = reftable_stack_reload(add->stack); + if (err) + goto done; + + if (!add->stack->disable_auto_compact) + err = reftable_stack_auto_compact(add->stack); + done: reftable_addition_close(add); return err; diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 52b4dc3b14..14a3fc11ee 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -289,6 +289,61 @@ static void test_reftable_stack_transaction_api(void) clear_dir(dir); } +static void test_reftable_stack_transaction_api_performs_auto_compaction(void) +{ + char *dir = get_tmp_dir(__LINE__); + struct reftable_write_options cfg = {0}; + struct reftable_addition *add = NULL; + struct reftable_stack *st = NULL; + int i, n = 20, err; + + err = reftable_new_stack(&st, dir, cfg); + EXPECT_ERR(err); + + for (i = 0; i <= n; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_SYMREF, + .value.symref = "master", + }; + char name[100]; + + snprintf(name, sizeof(name), "branch%04d", i); + ref.refname = name; + + /* + * Disable auto-compaction for all but the last runs. Like this + * we can ensure that we indeed honor this setting and have + * better control over when exactly auto compaction runs. + */ + st->disable_auto_compact = i != n; + + err = reftable_stack_new_addition(&add, st); + EXPECT_ERR(err); + + err = reftable_addition_add(add, &write_test_ref, &ref); + EXPECT_ERR(err); + + err = reftable_addition_commit(add); + EXPECT_ERR(err); + + reftable_addition_destroy(add); + + /* + * The stack length should grow continuously for all runs where + * auto compaction is disabled. When enabled, we should merge + * all tables in the stack. + */ + if (i != n) + EXPECT(st->merged->stack_len == i + 1); + else + EXPECT(st->merged->stack_len == 1); + } + + reftable_stack_destroy(st); + clear_dir(dir); +} + static void test_reftable_stack_validate_refname(void) { struct reftable_write_options cfg = { 0 }; @@ -1016,6 +1071,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_log_normalize); RUN_TEST(test_reftable_stack_tombstone); RUN_TEST(test_reftable_stack_transaction_api); + RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction); RUN_TEST(test_reftable_stack_update_index_check); RUN_TEST(test_reftable_stack_uptodate); RUN_TEST(test_reftable_stack_validate_refname); From patchwork Mon Dec 11 09:07:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486863 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="mQcKxdgu"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="t7rqlk3w" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B1A6AD9 for ; Mon, 11 Dec 2023 01:07:54 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id CF71A32026AA; Mon, 11 Dec 2023 04:07:53 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Mon, 11 Dec 2023 04:07:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285673; x=1702372073; bh=8v yfzJHgUfHLqxCEXW1N2lgEv94uuppecYrPzjqQO9g=; b=mQcKxdgunVBVUaUpvS 96ZqpeADwXbJsslxyNOIsk2QYshV7gHMxVmuwIlyTuDjUt88TCTtT/C36F85qxtB 6ZApNCVXipyB/yl6Xv21mpzNJFSo0cONG0NXL+AW3jfRNlqpmgJtkiM8+aRFnyZD RQMv1WdkARnln2X1YjcQ91qGFCsTpLuexxoYAT8kNNVEUgrWaU4vD6E7cXgWj+aU dZbHXsLXHB27fScOgR3aRVhZ1nZNEExoMY7vMV7uyctfKAhHI47Q5mmGxP91X/eY b6UGG1/w+V1WSFQ4Jmbe3NpKZxa5sou9AgiT/U7LPif/LU48nf+ATdN32mtomMNv loXg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285673; x=1702372073; bh=8vyfzJHgUfHLq xCEXW1N2lgEv94uuppecYrPzjqQO9g=; b=t7rqlk3wVTC3Y/kgXnCguqTeUs08H Azl0AYk+KkwbJUQ4Sr2H8s/JJDyjmOr5bisCGBUh9ys1BfEOA25ePSDboz5EiY5A sSD0gSqICSD2DUeG8TEPmnC80g7eyuivvLPzfAFetTgNd9U3K383FPlEbaVVG/F/ gDt+TXTgQxrldyclBSmzWcUeY2qE4eHv70z2kiwJo2zF1qXiKTF9BhnYOPoUt7dY 8mzuL4XwYAFPz+UBq+7pMUHhsGwcks4rYMVliR+ngmFdhMy0PhUVVKMz255vFXZ1 64C24Xx6F7A43xlnIhiJ2EDProlnHM/rmSTYAaOy2tqekTf3aDeeEjO9A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:52 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c84654e5 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:16 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:50 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 06/11] reftable/stack: reuse buffers when reloading stack Message-ID: <6ed9ba60db081a5210d325dabcbf232771754277.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In `reftable_stack_reload_once()` we iterate over all the tables added to the stack in order to figure out whether any of the tables needs to be reloaded. We use a set of buffers in this context to compute the paths of these tables, but discard those buffers on every iteration. This is quite wasteful given that we do not need to transfer ownership of the allocated buffer outside of the loop. Refactor the code to instead reuse the buffers to reduce the number of allocations we need to do. Note that we do not have to manually reset the buffer because `stack_filename()` does this for us already. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index f5d18a842a..2dd2373360 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -204,6 +204,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, reftable_calloc(sizeof(struct reftable_table) * names_len); int new_readers_len = 0; struct reftable_merged_table *new_merged = NULL; + struct strbuf table_path = STRBUF_INIT; int i; while (*names) { @@ -223,13 +224,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, if (!rd) { struct reftable_block_source src = { NULL }; - struct strbuf table_path = STRBUF_INIT; stack_filename(&table_path, st, name); err = reftable_block_source_from_file(&src, table_path.buf); - strbuf_release(&table_path); - if (err < 0) goto done; @@ -267,16 +265,13 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, for (i = 0; i < cur_len; i++) { if (cur[i]) { const char *name = reader_name(cur[i]); - struct strbuf filename = STRBUF_INIT; - stack_filename(&filename, st, name); + stack_filename(&table_path, st, name); reader_close(cur[i]); reftable_reader_free(cur[i]); /* On Windows, can only unlink after closing. */ - unlink(filename.buf); - - strbuf_release(&filename); + unlink(table_path.buf); } } @@ -288,6 +283,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, reftable_free(new_readers); reftable_free(new_tables); reftable_free(cur); + strbuf_release(&table_path); return err; } From patchwork Mon Dec 11 09:07:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486864 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="EyUuc6S9"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="QcU0Tcjx" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2F10ED9 for ; Mon, 11 Dec 2023 01:07:59 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 4A55432026C7; Mon, 11 Dec 2023 04:07:58 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 11 Dec 2023 04:07:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285677; x=1702372077; bh=d/ YwCty6KaZpaJE88FLpSCs/ki97O1H3EcjRPj3V1Is=; b=EyUuc6S95TXVom1QAC P52O4JXTikl8quxagK/EFPEfSV+ike94UXBcg9aU4+vwuwoz6ekRN1myQN6AMuVs 2wMfksFWVC0jle9pkXC25K1PKE2zFDMgmmlGSR/E5VsdKivHFXEqbExuLMhtnrrP dOqWCVnNS7Y0iYhGIl+C8IU9DfZpgvn6i5kB587bk8fXLfWb3Rc7m9Sawcp2yVJF kHKFanCH2NTOAdMMy5GYK2/qwXHA+PfqUkPvT2rM2VHl29/19wJgqjPXGqzFAFI6 MwLqX4wSoxT6jQbRxfSVnULM5dvP0W4PFulH5LVIICtMFofMlv6q7QFojYW86zX/ iFHg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285677; x=1702372077; bh=d/YwCty6KaZpa JE88FLpSCs/ki97O1H3EcjRPj3V1Is=; b=QcU0TcjxPuv+74APDyydch3uit1c4 viJCnCirs+mJur62qn+YSsu9OEnjzKXY8m1O805XS11c9anK2CCvrr14UgsjqK0y HyhWt8nCBFMygHfeOkhzHCzCpHBqqx8ziVEk1MhP+gJZ9yMOw+UXTFnDLBZOKPV9 JxlG+feQXGWqlxs7AvdHiPcwIxcDkyd0smXtdXzqzbaxlJ0AAStbd0Rn2OgziMHm uBBFQE2VkBw8ql++LnGgGxy7ctQi7Pc1Afku/fM3EJEgZyurUrIU3tO1ECcoirD4 MM6IUOH/d03hKl2Q6f0wkEyZcV/qMu4J5S1S3bk/T5csFu+RyUasogz8g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:56 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 688e2e20 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:20 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:54 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 07/11] reftable/stack: fix stale lock when dying Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When starting a transaction via `reftable_stack_init_addition()`, we create a lockfile for the reftable stack itself which we'll write the new list of tables to. But if we terminate abnormally e.g. via a call to `die()`, then we do not remove the lockfile. Subsequent executions of Git which try to modify references will thus fail with an out-of-date error. Fix this bug by registering the lock as a `struct tempfile`, which ensures automatic cleanup for us. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 47 +++++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 2dd2373360..0c235724e2 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -17,6 +17,8 @@ license that can be found in the LICENSE file or at #include "reftable-merged.h" #include "writer.h" +#include "tempfile.h" + static int stack_try_add(struct reftable_stack *st, int (*write_table)(struct reftable_writer *wr, void *arg), @@ -440,8 +442,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max) } struct reftable_addition { - int lock_file_fd; - struct strbuf lock_file_name; + struct tempfile *lock_file; struct reftable_stack *stack; char **new_tables; @@ -449,24 +450,19 @@ struct reftable_addition { uint64_t next_update_index; }; -#define REFTABLE_ADDITION_INIT \ - { \ - .lock_file_name = STRBUF_INIT \ - } +#define REFTABLE_ADDITION_INIT {0} static int reftable_stack_init_addition(struct reftable_addition *add, struct reftable_stack *st) { + struct strbuf lock_file_name = STRBUF_INIT; int err = 0; add->stack = st; - strbuf_reset(&add->lock_file_name); - strbuf_addstr(&add->lock_file_name, st->list_file); - strbuf_addstr(&add->lock_file_name, ".lock"); + strbuf_addf(&lock_file_name, "%s.lock", st->list_file); - add->lock_file_fd = open(add->lock_file_name.buf, - O_EXCL | O_CREAT | O_WRONLY, 0666); - if (add->lock_file_fd < 0) { + add->lock_file = create_tempfile(lock_file_name.buf); + if (!add->lock_file) { if (errno == EEXIST) { err = REFTABLE_LOCK_ERROR; } else { @@ -475,7 +471,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; } if (st->config.default_permissions) { - if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) { + if (chmod(add->lock_file->filename.buf, st->config.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; } @@ -495,6 +491,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, if (err) { reftable_addition_close(add); } + strbuf_release(&lock_file_name); return err; } @@ -512,15 +509,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables = NULL; add->new_tables_len = 0; - if (add->lock_file_fd > 0) { - close(add->lock_file_fd); - add->lock_file_fd = 0; - } - if (add->lock_file_name.len > 0) { - unlink(add->lock_file_name.buf); - strbuf_release(&add->lock_file_name); - } - + delete_tempfile(&add->lock_file); strbuf_release(&nm); } @@ -536,8 +525,10 @@ void reftable_addition_destroy(struct reftable_addition *add) int reftable_addition_commit(struct reftable_addition *add) { struct strbuf table_list = STRBUF_INIT; + int lock_file_fd = get_tempfile_fd(add->lock_file); int i = 0; int err = 0; + if (add->new_tables_len == 0) goto done; @@ -550,28 +541,20 @@ int reftable_addition_commit(struct reftable_addition *add) strbuf_addstr(&table_list, "\n"); } - err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len); + err = write_in_full(lock_file_fd, table_list.buf, table_list.len); strbuf_release(&table_list); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } - err = close(add->lock_file_fd); - add->lock_file_fd = 0; - if (err < 0) { - err = REFTABLE_IO_ERROR; - goto done; - } - - err = rename(add->lock_file_name.buf, add->stack->list_file); + err = rename_tempfile(&add->lock_file, add->stack->list_file); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } /* success, no more state to clean up. */ - strbuf_release(&add->lock_file_name); for (i = 0; i < add->new_tables_len; i++) { reftable_free(add->new_tables[i]); } From patchwork Mon Dec 11 09:07:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486865 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="MqviRA2E"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="epai8ouh" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 956A1DF for ; Mon, 11 Dec 2023 01:08:03 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id B38D332026EA; Mon, 11 Dec 2023 04:08:02 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Mon, 11 Dec 2023 04:08:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285682; x=1702372082; bh=o+ omBwsfZEekVbC9W2C9ko+HJ1hayNDHplnPVS05Bog=; b=MqviRA2EG5N9KE8cPV 2N38FsVRZBMppcEHhdtkEEDGN/VnFcG8VuaWuL7gfcaRIZeQ5klVsJObKBWryCEK b/Q6ItgQkOIID20VVc2ZtjVdfqPhOddLZWwQad4htnauy2ftOfjtOXa+cIIS+QpN bAIX7RtgWroCxpJ6fBvJnbYMtlhCUEDeFQXBJUY8/FvIVWX5D743vjJjmm+z9HsL oWeEeHsNw5nfyi32B8Rcm1iNZk5K/nIYP+Oy+pWQYrCpgZ34F16NAqP+MjA0iOat NL6Sfnb/SdRxhjZZFDU6v6u8/NAjGohJrBkFGolmSmIgND/9Bw6YC+AdD4Tah6+b B2DQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285682; x=1702372082; bh=o+omBwsfZEekV bC9W2C9ko+HJ1hayNDHplnPVS05Bog=; b=epai8ouhCANSHqXI8Lq/KCQyu/6NZ 78AXbpCKNGWRIbwTWhchH2QY8EiQ2Gbu/Q3IvVHh8bPxCOGtg8MzmLPQDYVnfPPK 4j5zxJsUCKFD5bb2QsBQT4LKZk8PUlTfxip339GJRdZwzOytIYm8gaY8WpHFjR2U 1cfGO81i1datqLMS6vJx/PS7i+Q6+sVB0+174Jx+mEALrPMvy7e5UZ8RluNzIrRA ZkPQUKimnQvO9Km8LFGPltIyu70toicKRiqBA2kw+WoWhiNqoOZQd33eYol6ufEF NmxEATVbnS1W7ZZChJFHo7gbdHx8t5t+fsVlT8lbmW5wMSr5v7mbtcpHQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeegvedtffehvdehheegueeuffefteeitdefheejheetveeiheekkeeifefhvdev heenucffohhmrghinhepuhhpuggrthgvrdhnvgifnecuvehluhhsthgvrhfuihiivgeptd enucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:08:01 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id da6ce7fe (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:24 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:59 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 08/11] reftable/stack: fix use of unseeded randomness Message-ID: <5598460b8112e20d5f8a3889a482732d9475e6a7.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When writing a new reftable stack, Git will first create the stack with a random suffix so that concurrent updates will not try to write to the same file. This random suffix is computed via a call to rand(3P). But we never seed the function via srand(3P), which means that the suffix is in fact always the same. Fix this bug by using `git_rand()` instead, which does not need to be initialized. While this function is likely going to be slower depending on the platform, this slowness should not matter in practice as we only use it when writing a new reftable stack. Signed-off-by: Patrick Steinhardt --- reftable/readwrite_test.c | 6 +++--- reftable/stack.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 469ab79a5a..278663f22d 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -141,8 +141,8 @@ static void test_log_buffer_size(void) */ uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ]; for (i = 0; i < GIT_SHA1_RAWSZ; i++) { - hash1[i] = (uint8_t)(rand() % 256); - hash2[i] = (uint8_t)(rand() % 256); + hash1[i] = (uint8_t)(git_rand() % 256); + hash2[i] = (uint8_t)(git_rand() % 256); } log.value.update.old_hash = hash1; log.value.update.new_hash = hash2; @@ -320,7 +320,7 @@ static void test_log_zlib_corruption(void) }; for (i = 0; i < sizeof(message) - 1; i++) - message[i] = (uint8_t)(rand() % 64 + ' '); + message[i] = (uint8_t)(git_rand() % 64 + ' '); reftable_writer_set_limits(w, 1, 1); diff --git a/reftable/stack.c b/reftable/stack.c index 0c235724e2..16bab82063 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -434,7 +434,7 @@ int reftable_stack_add(struct reftable_stack *st, static void format_name(struct strbuf *dest, uint64_t min, uint64_t max) { char buf[100]; - uint32_t rnd = (uint32_t)rand(); + uint32_t rnd = (uint32_t)git_rand(); snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x", min, max, rnd); strbuf_reset(dest); From patchwork Mon Dec 11 09:08:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486866 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="k/OCUC71"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="IQDpcH9y" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 12DDCD9 for ; Mon, 11 Dec 2023 01:08:08 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 2FC8A3202709; Mon, 11 Dec 2023 04:08:07 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Mon, 11 Dec 2023 04:08:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285686; x=1702372086; bh=gU 91c3tl+qRFFG28uKLIsZD1PebJPVL/SEfpBfQIq2U=; b=k/OCUC719EmKlSae0n ZKpqxhj7iz9iMCZwFevjxWHxpAg3mwH0YHSQw/kuKborHgZHTJYQIGpVSD+SDtP4 rOJgRMT35rQVb+M0SC70cDzrG8mZaC/hGzQoZyqdBilXRd3B8FvtBEGU3hr0VibN ZEBOZsOx7g+7NRXvidBxA2GGwKeQ7nWIJOvyvt5S7AADy0j/g359hRollHeEFHrH w1H4MmR9+nBwvwbxqgz/126MbuMqpF7NQ2IjhDyE/rBQrrSuBc19UxjjEv8jcF/Z ffraxbA0nQrs/4uall7dbAtQK4b3GxjsVV4yblA9gMHRXEnJu5WAY8YhD3gqJsTG BX2g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285686; x=1702372086; bh=gU91c3tl+qRFF G28uKLIsZD1PebJPVL/SEfpBfQIq2U=; b=IQDpcH9yW0LNYcgbtHtBVgNysmwF/ vxxdq9iHOUHkyS5C5+3bNQegjN74bG0fizmHT87bnZJUv2rU/KJDQhRmBVvb5GDm hFp+HueoKPzFSeW1KT3xaKongO7UAxFeTtEpmcY3aKGLxZbeyd8M8rPpVO+ImgOf ieNtp46ZbM1k4ubCOaaqZan2FQISpAnzododcxZk4K14SPjGCoflQ/NvW4dyyp85 +LKS2A1FlYEUk5dxRd2sb4uNFyrZsN4LOk8jGzxa8rDsm20dYBV00mrqxwD2nIW4 mVJ7rZcyEwkd9X6Q6GHplos1WQRfpup8jSWSujCsi7XCxnQ4Md2TXIIpA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:08:05 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 8573ddf0 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:29 +0000 (UTC) Date: Mon, 11 Dec 2023 10:08:03 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 09/11] reftable/merged: reuse buffer to compute record keys Message-ID: <79e03826039e9b91e456e4c7d2c5a2367a082736.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When iterating over entries in the merged iterator's queue, we compute the key of each of the entries and write it into a buffer. We do not reuse the buffer though and thus re-allocate it on every iteration, which is wasteful given that we never transfer ownership of the allocated bytes outside of the loop. Refactor the code to reuse the buffer. This also fixes a potential memory leak when `merged_iter_advance_subiter()` returns an error. Signed-off-by: Patrick Steinhardt --- reftable/merged.c | 31 ++++++++++++++++--------------- reftable/merged.h | 2 ++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index 5ded470c08..556bb5c556 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -52,6 +52,8 @@ static void merged_iter_close(void *p) reftable_iterator_destroy(&mi->stack[i]); } reftable_free(mi->stack); + strbuf_release(&mi->key); + strbuf_release(&mi->entry_key); } static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi, @@ -85,7 +87,6 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx) static int merged_iter_next_entry(struct merged_iter *mi, struct reftable_record *rec) { - struct strbuf entry_key = STRBUF_INIT; struct pq_entry entry = { 0 }; int err = 0; @@ -105,33 +106,31 @@ static int merged_iter_next_entry(struct merged_iter *mi, such a deployment, the loop below must be changed to collect all entries for the same key, and return new the newest one. */ - reftable_record_key(&entry.rec, &entry_key); + reftable_record_key(&entry.rec, &mi->entry_key); while (!merged_iter_pqueue_is_empty(mi->pq)) { struct pq_entry top = merged_iter_pqueue_top(mi->pq); - struct strbuf k = STRBUF_INIT; - int err = 0, cmp = 0; + int cmp = 0; - reftable_record_key(&top.rec, &k); + reftable_record_key(&top.rec, &mi->key); - cmp = strbuf_cmp(&k, &entry_key); - strbuf_release(&k); - - if (cmp > 0) { + cmp = strbuf_cmp(&mi->key, &mi->entry_key); + if (cmp > 0) break; - } merged_iter_pqueue_remove(&mi->pq); err = merged_iter_advance_subiter(mi, top.index); - if (err < 0) { - return err; - } + if (err < 0) + goto done; reftable_record_release(&top.rec); } reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id)); + +done: reftable_record_release(&entry.rec); - strbuf_release(&entry_key); - return 0; + strbuf_release(&mi->entry_key); + strbuf_release(&mi->key); + return err; } static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec) @@ -248,6 +247,8 @@ static int merged_table_seek_record(struct reftable_merged_table *mt, .typ = reftable_record_type(rec), .hash_id = mt->hash_id, .suppress_deletions = mt->suppress_deletions, + .key = STRBUF_INIT, + .entry_key = STRBUF_INIT, }; int n = 0; int err = 0; diff --git a/reftable/merged.h b/reftable/merged.h index 7d9f95d27e..d5b39dfe7f 100644 --- a/reftable/merged.h +++ b/reftable/merged.h @@ -31,6 +31,8 @@ struct merged_iter { uint8_t typ; int suppress_deletions; struct merged_iter_pqueue pq; + struct strbuf key; + struct strbuf entry_key; }; void merged_table_release(struct reftable_merged_table *mt); From patchwork Mon Dec 11 09:08:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486867 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="OYJWWmws"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="HZOlT5rS" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8996B100 for ; Mon, 11 Dec 2023 01:08:12 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id A3DB93202728; Mon, 11 Dec 2023 04:08:11 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Mon, 11 Dec 2023 04:08:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285691; x=1702372091; bh=jp /Yhc9QXyPlx+0Sedzvr6+U5MuE3ip3mgMoUgceSr0=; b=OYJWWmwsHvmp7gy5Ts 046OgW89/CqxdVzWL/e+BzfMSukfCT5GJuseR6IxYlyenXP0Zg91att/71s1SxNB go2zw97XQnFMADvWQNaNw2j40VzDzayPxnx+OUSRopws5H2a/EcelxU4ZQp8WQX5 NanV35FaCWkFvMnUgwc8/T9FI2IEu+rF4XmIj8Kvq0BH6R6VGD0cU4v6XRtp/gV9 F2ETnYWgeW6tA1e8E9RyDS+LrPxjjj1U7x9ZclDSjBz/RJq/LEjq91368optmHP6 h02otLy3dTx6BThBoDXcqFM9JsBoUaH9um2dOdemfcPUbdFXcH2kIMb4KdjucLmf yt8w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285691; x=1702372091; bh=jp/Yhc9QXyPlx +0Sedzvr6+U5MuE3ip3mgMoUgceSr0=; b=HZOlT5rSA0R1lXDLmbVP8Cw+UvlU4 jR2TtSET+Cud326FlGOvpNZxHeF+7FbIm+gZLXvk0GhXNebQXKcUPDfS3nATlD9W IMbg+9Vh66tPqPQUlR4i8y0qhB3ut15faJP3x5Vmcd+WMHIvvlRqUCH1dr4QEr4e Ni25xEvTJrijyVv/VG46xRUQuRjM7n16ysQc3KQ4ybLhKytJtJzx9k85TbCi6NIP YCEJ1qo98FigeZCQrM5zOsyRXWgJeZv6xAvTJXj8EOte8rBY4FIRnVqIsWMzEwMx 6ZnFjEcr4pJmjC5xGXD3B/jkA5ev3FAVkp6zc8pHKfbW352xBV4L73ORw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:08:09 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 51867a61 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:33 +0000 (UTC) Date: Mon, 11 Dec 2023 10:08:07 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 10/11] reftable/block: introduce macro to initialize `struct block_iter` Message-ID: <8574ad7635614fcc64c23ac3bb464f460768c7f3.1702285387.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: There are a bunch of locations where we initialize members of `struct block_iter`, which makes it harder than necessary to expand this struct to have additional members. Unify the logic via a new `BLOCK_ITER_INIT` macro that initializes all members. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 4 +--- reftable/block.h | 4 ++++ reftable/block_test.c | 4 ++-- reftable/iter.h | 8 ++++---- reftable/reader.c | 7 +++---- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 34d4d07369..8c6a8c77fc 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -389,9 +389,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, struct reftable_record rec = reftable_new_record(block_reader_type(br)); struct strbuf key = STRBUF_INIT; int err = 0; - struct block_iter next = { - .last_key = STRBUF_INIT, - }; + struct block_iter next = BLOCK_ITER_INIT; int i = binsearch(br->restart_count, &restart_key_less, &args); if (args.error) { diff --git a/reftable/block.h b/reftable/block.h index 87c77539b5..51699af233 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -86,6 +86,10 @@ struct block_iter { struct strbuf last_key; }; +#define BLOCK_ITER_INIT { \ + .last_key = STRBUF_INIT, \ +} + /* initializes a block reader. */ int block_reader_init(struct block_reader *br, struct reftable_block *bl, uint32_t header_off, uint32_t table_block_size, diff --git a/reftable/block_test.c b/reftable/block_test.c index cb88af4a56..c00bbc8aed 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -32,7 +32,7 @@ static void test_block_read_write(void) int i = 0; int n; struct block_reader br = { 0 }; - struct block_iter it = { .last_key = STRBUF_INIT }; + struct block_iter it = BLOCK_ITER_INIT; int j = 0; struct strbuf want = STRBUF_INIT; @@ -87,7 +87,7 @@ static void test_block_read_write(void) block_iter_close(&it); for (i = 0; i < N; i++) { - struct block_iter it = { .last_key = STRBUF_INIT }; + struct block_iter it = BLOCK_ITER_INIT; strbuf_reset(&want); strbuf_addstr(&want, names[i]); diff --git a/reftable/iter.h b/reftable/iter.h index 09eb0cbfa5..47d67d84df 100644 --- a/reftable/iter.h +++ b/reftable/iter.h @@ -53,10 +53,10 @@ struct indexed_table_ref_iter { int is_finished; }; -#define INDEXED_TABLE_REF_ITER_INIT \ - { \ - .cur = { .last_key = STRBUF_INIT }, .oid = STRBUF_INIT, \ - } +#define INDEXED_TABLE_REF_ITER_INIT { \ + .cur = BLOCK_ITER_INIT, \ + .oid = STRBUF_INIT, \ +} void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it, struct indexed_table_ref_iter *itr); diff --git a/reftable/reader.c b/reftable/reader.c index b4db23ce18..9de64f50b4 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -224,10 +224,9 @@ struct table_iter { struct block_iter bi; int is_finished; }; -#define TABLE_ITER_INIT \ - { \ - .bi = {.last_key = STRBUF_INIT } \ - } +#define TABLE_ITER_INIT { \ + .bi = BLOCK_ITER_INIT \ +} static void table_iter_copy_from(struct table_iter *dest, struct table_iter *src) From patchwork Mon Dec 11 09:08:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486868 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="O1ut0xgV"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="PqasaEwF" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 11813EB for ; Mon, 11 Dec 2023 01:08:16 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 239A43202738; Mon, 11 Dec 2023 04:08:15 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 11 Dec 2023 04:08:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285694; x=1702372094; bh=GV ycWQ8hCxswlDagTpg3H6ecLfcmhkzitmNJa9fGJ4U=; b=O1ut0xgVlHRQ+1IYIR iUJDDdOMonsx5LicQN+xqr4hBPtRq4JJ5Nj8iTVAart1jCabShduhydTZpMFuihl dLR1u0vA1i74DhXLkzg1HrfI1v2F6BdFnqW4v+I3MfzY09UwtM5n5x5i/ViQ6tld YFpv83cP3oDUPKejWC5oa0MdzvByDQvb7lI7y2RbGLLqalk3FZCsr4SBZ18kLa70 jTlDruLvNFZaIbX3PpR7zAIzkouVbAugX7jjSYbVym9BiyFGMQ4p4FDvRZpM8Rvw QnztNl56kTD/WEDQ9pBkliVJQ7xOj2kyBli8mBXr3p/MNcsQk9FcWH+Nw1P7gj1X 0J4A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285694; x=1702372094; bh=GVycWQ8hCxswl DagTpg3H6ecLfcmhkzitmNJa9fGJ4U=; b=PqasaEwFRKjmDyQIjxR8djRdhzsKd cPP6JbS7R6v5N+sop8qf1wFXWGuUycDJgwjjsoUcj1YJgLrdkDQPgCEgORfMtCaI ICvgnavQoYVFPOrlnOelbg5xxO34jvBLdK6B9OGEQ/jxaOHrQo92ueaiJZ8SbXuq xKOl7pCSKb26e52vFCeu/LjWwp2djoGXSTHlFmtRGPF42+7pIEczjvaYiJBOoLxY ZZwotbQG6GEK03GvGWO/CkqCUQ6N/zQ031Bv1UJ8a46zKas2VcCB9y2VyoMT1vxn f7kwJ8RVH9+VYqPmIaSTDgSh1MNe/zPxXxj2A+LrHoCnf5vv1+6mH/Gqw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpeefnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:08:13 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1c27f6fd (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:37 +0000 (UTC) Date: Mon, 11 Dec 2023 10:08:12 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 11/11] reftable/block: reuse buffer to compute record keys Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When iterating over entries in the block iterator we compute the key of each of the entries and write it into a buffer. We do not reuse the buffer though and thus re-allocate it on every iteration, which is wasteful. Refactor the code to reuse the buffer. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 19 ++++++++----------- reftable/block.h | 2 ++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 8c6a8c77fc..1df3d8a0f0 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -323,30 +323,28 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec) .len = it->br->block_len - it->next_off, }; struct string_view start = in; - struct strbuf key = STRBUF_INIT; uint8_t extra = 0; int n = 0; if (it->next_off >= it->br->block_len) return 1; - n = reftable_decode_key(&key, &extra, it->last_key, in); + n = reftable_decode_key(&it->key, &extra, it->last_key, in); if (n < 0) return -1; - if (!key.len) + if (!it->key.len) return REFTABLE_FORMAT_ERROR; string_view_consume(&in, n); - n = reftable_record_decode(rec, key, extra, in, it->br->hash_size); + n = reftable_record_decode(rec, it->key, extra, in, it->br->hash_size); if (n < 0) return -1; string_view_consume(&in, n); strbuf_reset(&it->last_key); - strbuf_addbuf(&it->last_key, &key); + strbuf_addbuf(&it->last_key, &it->key); it->next_off += start.len - in.len; - strbuf_release(&key); return 0; } @@ -377,6 +375,7 @@ int block_iter_seek(struct block_iter *it, struct strbuf *want) void block_iter_close(struct block_iter *it) { strbuf_release(&it->last_key); + strbuf_release(&it->key); } int block_reader_seek(struct block_reader *br, struct block_iter *it, @@ -387,7 +386,6 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, .r = br, }; struct reftable_record rec = reftable_new_record(block_reader_type(br)); - struct strbuf key = STRBUF_INIT; int err = 0; struct block_iter next = BLOCK_ITER_INIT; @@ -414,8 +412,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, if (err < 0) goto done; - reftable_record_key(&rec, &key); - if (err > 0 || strbuf_cmp(&key, want) >= 0) { + reftable_record_key(&rec, &it->key); + if (err > 0 || strbuf_cmp(&it->key, want) >= 0) { err = 0; goto done; } @@ -424,8 +422,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, } done: - strbuf_release(&key); - strbuf_release(&next.last_key); + block_iter_close(&next); reftable_record_release(&rec); return err; diff --git a/reftable/block.h b/reftable/block.h index 51699af233..17481e6331 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -84,10 +84,12 @@ struct block_iter { /* key for last entry we read. */ struct strbuf last_key; + struct strbuf key; }; #define BLOCK_ITER_INIT { \ .last_key = STRBUF_INIT, \ + .key = STRBUF_INIT, \ } /* initializes a block reader. */