From patchwork Fri May 10 10:29:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13661274 Received: from wfhigh1-smtp.messagingengine.com (wfhigh1-smtp.messagingengine.com [64.147.123.152]) (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 03A64168B01 for ; Fri, 10 May 2024 10:29:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.152 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715336971; cv=none; b=AfFCEGEVGU2jZ6v/214KucpSiXcsukF7jwS3oIvsiOX35WLI2pN6dqbJGP0Jrmp/1ihzdKV7gKH16rD8SRFGLDr0zf7wdm1FYmrLbtpYEElCKyTxz3MGEaRwsG0e6MqQaSfzBLJjQjOIkGcX3THUbccjzBTj6zR1Bw40IUYkHa8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715336971; c=relaxed/simple; bh=gu/fthYt8H/q7rhSnojtHqzpVupq2uxZt+5jzmWFh7c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HCBBL/VehboUbMHGtTnusLZtkmAjs2jCG7eJ5aBAEkvyDh3jMOCmyVZnIGxknJ5de46waPI58OXVRucVV+YhnvWGiS9n6WxLEnN94B5ofSmrJUJb7rIZ92pTgJEyzyDGfow6RVwHU9/fO+f56qUa9guhymdNhsT128KZSC5y77M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject 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=CQEKNUsl; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=e0VKwEuh; arc=none smtp.client-ip=64.147.123.152 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject 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="CQEKNUsl"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="e0VKwEuh" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.west.internal (Postfix) with ESMTP id 1AA31180009A; Fri, 10 May 2024 06:29:29 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Fri, 10 May 2024 06:29:29 -0400 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:subject :subject:to:to; s=fm3; t=1715336968; x=1715423368; bh=3d0FFhFQHf INkXefT8jArDIFCWHcBC6W3EEMM8bSXlA=; b=CQEKNUslGxhrNNxUmU7/SR6bQb Y8brxzBFCmedZpQioPBzECxYV/nqusrn79AX4GpSPzC1FVhKvO6pjLirc4GNfNxf IEkUWPe0dS2LWv/IlgQo5lzXn+9eO3tfRV6hoEJqfNtiKydQi6DA68Tr46lIMfK/ 1CGhM+Dp7fNho+edmUyNi7JRhWgypJvESpO+OGSbdGPqaq94uxywH/X44DZBxhwG keW3DU/SIqnxQL+18Bz8rtBejaqBJcr0zEtmkMferd8cIQEsTvg6hHceKHj4cvku O8YfX2ZylgjwZP7WTVkPs11kIeklzGlvYiyU7Ttufbl2wvHVQfPYGVUhtalQ== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1715336968; x=1715423368; bh=3d0FFhFQHfINkXefT8jArDIFCWHc BC6W3EEMM8bSXlA=; b=e0VKwEuhfV7HD+DbegAaTO4z3Pz9yzprQXK0JNtwQYaW zmsfDjzJsc/5sb8LQNKSVB1jBSmCkV/bckLreXeVI9ZkVQj3usWPHm8DrklAQGZR w9xopBw/7Y5D6mzNMeVb6CK4y8wdQx/RJSQpKD+uXzVvWR7lu643aBaid5RT+Lci LRGVaMmUOo/Fz4tbBOIfIfsUkwOktFTgCmLa2YRPcAnLSZICiOXJJzBbZ/UlByhB FRFzYJsjPmWJMXs7Xchpe2yl2oofowidP4IRX1yn8f8ugChPCpgTGTWoymRrt7nu FNOOaRtgwioW8WzqQNt2NKgaIMTF2xRnKOWQsEnsmQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvdefkedgvdekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 10 May 2024 06:29:27 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 859208f4 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 10 May 2024 10:29:15 +0000 (UTC) Date: Fri, 10 May 2024 12:29:25 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Karthik Nayak , Justin Tobler Subject: [PATCH v2 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);