From patchwork Thu May 2 06:51:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13651324 Received: from fhigh8-smtp.messagingengine.com (fhigh8-smtp.messagingengine.com [103.168.172.159]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DE7741CAA5 for ; Thu, 2 May 2024 06:51:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.159 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714632703; cv=none; b=p8CjjckDRXsv7Exbvm+dvalVh1m0/QF9zBTtBzFAGdJVVVwIX2T15NSpbBuNUN0JmC9XUNlgFslPXH16iKMZNZz3ArCEraqx14CvdU69SknuaFm3eZ3duhKGS7EjweB6CoRj6//eiGxgY7ZXkGpVO+3/IeOTawlgQ8g2x5EMx6c= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714632703; c=relaxed/simple; bh=ynRGB7lJG8sNhHFo5sRXSb6Apy316AbP2dmhNzAiKXY=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KzDEe6maDU29qHzsNug3Ul4sGvnsB1Cp3h9V2YiCP7ftO283dzuNBnjY+O89ZaXIBZQqMC6L4Lve4oeYAEvxCuEH6ytNNmKvogsSJjp000x3Tk0vT4VzTaZFz9k6GVBDRB8tKdaCtg5qTXBSOc+gsd10h9LtDakeo/AatHv2mW4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=ocq5LfwY; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=VkiaQNhE; arc=none smtp.client-ip=103.168.172.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="ocq5LfwY"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="VkiaQNhE" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id D141311400CA for ; Thu, 2 May 2024 02:51:40 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Thu, 02 May 2024 02:51:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1714632700; x=1714719100; bh=OIC2Jw9GSe kd3bCME2UpPW8YcZmAwwuASVSTATkHWvQ=; b=ocq5LfwYQwfE/JK6xjdYNjX7xK 01mx/EMpp3giQoofjd5vflgLcX+uVYMMbkgkMIpVXN2e29bNQzWZl/wnoSyHdDk9 bbUWfGIlMOTX1+bx+rLT7bBzmG/O6BLYyR+XSu7kv76A584JIbTb7Apg8asLy46w F67Bk1Po+opcUXrt/1GEhQ28QElNnX5P9Sqgt0R14Dx47C/hi5ulXU83Ipv8vQla UsPNZrza2jRCmlEdej1/eAETEDdXbxrTmaAuDYnSiHPEunMbCdTAi8D8JuP7UaTJ Su0+CRJ+qJON4w7xsaZpXVemd8Esr3qrxPajBJUyDaVaPgEhCwBu3Xd2fFaw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1714632700; x=1714719100; bh=OIC2Jw9GSekd3bCME2UpPW8YcZmA wwuASVSTATkHWvQ=; b=VkiaQNhE6AAdppSpXq3vnG5TMilGM0lRkHQbAO0FJTyZ pSIMgUw/wIFxL0QlynmcGazoSM9Vd1ZWpvXr21mJH5z1X1gKnbPWA22w2I7o6AGv 4GU8LsNs+BZS8OUPVeIsEppun11xFdMf6+zXZ0R0bfOAdeuBgCQlsBRVVKE+6XBK h8UDFxxWfkIcuwP+PQUuwrbLMGqk/F+KnDwEijTzfK3trQBmd0uG+6mpWniysbLb IIjl5a0EPnpCk/8j100P1r145QFBnRWPMBK0S32kLwRjKcgiFZPmoQC8MbjjhSfr v/P6EoS4NuaygXtDu1mtINzl6jQmDZ3IWzq941iWrQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvddujedgudduudcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 2 May 2024 02:51:40 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id e628a170 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Thu, 2 May 2024 06:51:14 +0000 (UTC) Date: Thu, 2 May 2024 08:51:37 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 02/11] reftable: consistently pass write opts as value 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: We sometimes pass the refatble write options as value and sometimes as a pointer. This is quite confusing and makes the reader wonder whether the options get modified sometimes. In fact, `reftable_new_writer()` does cause the caller-provided options to get updated when some values aren't set up. This is quite unexpected, but didn't cause any harm until now. Refactor the code to consistently pass the options as a value so that they are local to the subsystem they are being passed into so that we can avoid weirdness like this. Signed-off-by: Patrick Steinhardt --- reftable/merged_test.c | 6 +++--- reftable/readwrite_test.c | 26 +++++++++++++------------- reftable/refname_test.c | 2 +- reftable/reftable-writer.h | 2 +- reftable/stack.c | 4 ++-- reftable/writer.c | 12 +++++++----- 6 files changed, 27 insertions(+), 25 deletions(-) diff --git a/reftable/merged_test.c b/reftable/merged_test.c index 530fc82d1c..4ac81de8d4 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -42,7 +42,7 @@ static void write_test_table(struct strbuf *buf, } } - w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts); + w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, opts); reftable_writer_set_limits(w, min, max); for (i = 0; i < n; i++) { @@ -70,7 +70,7 @@ static void write_test_log_table(struct strbuf *buf, .exact_log_message = 1, }; struct reftable_writer *w = NULL; - w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts); + w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, opts); reftable_writer_set_limits(w, update_index, update_index); for (i = 0; i < n; i++) { @@ -403,7 +403,7 @@ static void test_default_write_opts(void) struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record rec = { .refname = "master", diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index a6dbd214c5..27631a041b 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -51,7 +51,7 @@ static void write_table(char ***names, struct strbuf *buf, int N, .hash_id = hash_id, }; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, buf, opts); struct reftable_ref_record ref = { NULL }; int i = 0, n; struct reftable_log_record log = { NULL }; @@ -129,7 +129,7 @@ static void test_log_buffer_size(void) .message = "commit: 9\n", } } }; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); /* This tests buffer extension for log compression. Must use a random hash, to ensure that the compressed part is larger than the original. @@ -172,7 +172,7 @@ static void test_log_overflow(void) }, }; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); memset(msg, 'x', sizeof(msg) - 1); reftable_writer_set_limits(w, update_index, update_index); @@ -199,7 +199,7 @@ static void test_log_write_read(void) struct reftable_block_source source = { NULL }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); const struct reftable_stats *stats = NULL; reftable_writer_set_limits(w, 0, N); for (i = 0; i < N; i++) { @@ -288,7 +288,7 @@ static void test_log_zlib_corruption(void) struct reftable_block_source source = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); const struct reftable_stats *stats = NULL; char message[100] = { 0 }; int err, i, n; @@ -526,7 +526,7 @@ static void test_table_refs_for(int indexed) struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_iterator it = { NULL }; int j; @@ -619,7 +619,7 @@ static void test_write_empty_table(void) struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_block_source source = { NULL }; struct reftable_reader *rd = NULL; struct reftable_ref_record rec = { NULL }; @@ -657,7 +657,7 @@ static void test_write_object_id_min_length(void) }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record ref = { .update_index = 1, .value_type = REFTABLE_REF_VAL1, @@ -692,7 +692,7 @@ static void test_write_object_id_length(void) }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record ref = { .update_index = 1, .value_type = REFTABLE_REF_VAL1, @@ -726,7 +726,7 @@ static void test_write_empty_key(void) struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record ref = { .refname = "", .update_index = 1, @@ -749,7 +749,7 @@ static void test_write_key_order(void) struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record refs[2] = { { .refname = "b", @@ -792,7 +792,7 @@ static void test_write_multiple_indices(void) struct reftable_reader *reader; int err, i; - writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, &opts); + writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, opts); reftable_writer_set_limits(writer, 1, 1); for (i = 0; i < 100; i++) { struct reftable_ref_record ref = { @@ -869,7 +869,7 @@ static void test_write_multi_level_index(void) struct reftable_reader *reader; int err; - writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, &opts); + writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, opts); reftable_writer_set_limits(writer, 1, 1); for (size_t i = 0; i < 200; i++) { struct reftable_ref_record ref = { diff --git a/reftable/refname_test.c b/reftable/refname_test.c index b9cc62554e..3468253be7 100644 --- a/reftable/refname_test.c +++ b/reftable/refname_test.c @@ -30,7 +30,7 @@ static void test_conflict(void) struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = - reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, opts); struct reftable_ref_record rec = { .refname = "a/b", .value_type = REFTABLE_REF_SYMREF, diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h index 155bf0bbe2..44cb986465 100644 --- a/reftable/reftable-writer.h +++ b/reftable/reftable-writer.h @@ -92,7 +92,7 @@ struct reftable_stats { struct reftable_writer * reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t), int (*flush_func)(void *), - void *writer_arg, struct reftable_write_options *opts); + void *writer_arg, struct reftable_write_options opts); /* Set the range of update indices for the records we will add. When writing a table into a stack, the min should be at least diff --git a/reftable/stack.c b/reftable/stack.c index 3979657793..7b4fff7c9e 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -767,7 +767,7 @@ int reftable_addition_add(struct reftable_addition *add, tab_fd = get_tempfile_fd(tab_file); wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, - &add->stack->opts); + add->stack->opts); err = write_table(wr, arg); if (err < 0) goto done; @@ -861,7 +861,7 @@ static int stack_compact_locked(struct reftable_stack *st, } wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, - &tab_fd, &st->opts); + &tab_fd, st->opts); err = stack_write_compact(st, wr, first, last, config); if (err < 0) goto done; diff --git a/reftable/writer.c b/reftable/writer.c index 1d9ff0fbfa..ad2f2e6c65 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -122,20 +122,22 @@ static struct strbuf reftable_empty_strbuf = STRBUF_INIT; struct reftable_writer * reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t), int (*flush_func)(void *), - void *writer_arg, struct reftable_write_options *opts) + void *writer_arg, struct reftable_write_options opts) { struct reftable_writer *wp = reftable_calloc(1, sizeof(*wp)); strbuf_init(&wp->block_writer_data.last_key, 0); - options_set_defaults(opts); - if (opts->block_size >= (1 << 24)) { + + options_set_defaults(&opts); + if (opts.block_size >= (1 << 24)) { /* TODO - error return? */ abort(); } + wp->last_key = reftable_empty_strbuf; - REFTABLE_CALLOC_ARRAY(wp->block, opts->block_size); + REFTABLE_CALLOC_ARRAY(wp->block, opts.block_size); wp->write = writer_func; wp->write_arg = writer_arg; - wp->opts = *opts; + wp->opts = opts; wp->flush = flush_func; writer_reinit_block_writer(wp, BLOCK_TYPE_REF);