From patchwork Fri Aug 23 14:12:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775279 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 3FCCE18890D for ; Fri, 23 Aug 2024 14:12:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422356; cv=none; b=gTeGUa4gTncZ+l04uUpbWwnrHQ1gnsEpyqcS5y3JMM2oTuPuWDFCbGFO0q0qMkjvvK00jmBqdJ5mQ0MtJ/krgA9vbeHW6nFD01IGA4ui6CAFKiP+iwmujKKLKU/KEGVmR2Ca1L5zWggCWyLwUP6Rq/VO+1CWS3kVBR5zHI5T3aM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422356; c=relaxed/simple; bh=0yRNSqd6ZUFg+q54E+wI645F5+3u7qNqXSjEfe3nV8s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SeeMkmtzqun5xkrr5JKZH3q1d8kLZjNyBD9CDRca/5uswCq/IsNoBoL1X8Ctf+IO8UZkLl50bsTli8EaLcvSIdeFhMBhtD3jOnD9V6zIt/iDl0PrwVhIXp0vpTW4OntQPkcNxt/6yTkqzyrl18hYfChJep1vDrWObMrdF9OwUPE= 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=B/a6NLXb; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=MIzxQq5o; arc=none smtp.client-ip=103.168.172.145 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="B/a6NLXb"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="MIzxQq5o" Received: from phl-compute-05.internal (phl-compute-05.nyi.internal [10.202.2.45]) by mailfout.nyi.internal (Postfix) with ESMTP id 24957138FFC4; Fri, 23 Aug 2024 10:12:32 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Fri, 23 Aug 2024 10:12:32 -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=fm1; t=1724422352; x=1724508752; bh=o2pS6SKjr2 fmp194IidRXafb3pzSTJdz4YHx6jmYyxQ=; b=B/a6NLXb0wAbsb/pvYQaqGahn+ r6zZq7Tj5cyjXKt/onf+DsbQr8L5AyH1pWZDrDcvBEyNPYlqioODWxOqFPwFgY1u tGaMXPK2qtSUB55Q7ZCE6oGho3I4ha9cfIFDAEfKbzZOZKBgVDNGqwMv7C+dXyYj GnJucSCYfOo+16GzzG7WYMrjXn4T4s02GaEVVrOfRq3TH21bviEmm7F0pahpzFsF GcK/4+jhx7/aJQWsLblqgWN1+GC9EHJxfCkcWd3g8WEhUMBpLo3LalTsbHFrmVeY Kr8GZWsjfdnkEv+1yKHwW8tTAsBB42GEdTkUtbNzu52VaEOCzHhaMR49uD3Q== 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= fm1; t=1724422352; x=1724508752; bh=o2pS6SKjr2fmp194IidRXafb3pzS TJdz4YHx6jmYyxQ=; b=MIzxQq5o4xDCemCOuAZf0kEpkOYq9+n3E3ELZHzd/w8q /9ed8V0Xauf3wza7rNtISNriMY7jHNLRhY0FkgPu5LiJx8sVqITCCaMmn7jyhF4c LubAVT9VO6MJn7ZaGgu+54Ox8wfNHXpC6/SdfrYLIvIuuPz3UATY9pBTdawDN4nZ araNJly3BcUXZYi3dGfKzbfczVaSiVnAcxgHzOugX8DPqrjT143sFUlgkJcDMqru xZcIKwTNGgvms0ZlJMTM/Z1jB4njufICGj6pa1WMp9TPwudYaDOzp+HCHa7XaNb4 jVHgX7ftjCyVO9zyqwgA9cwT9auYY3ohrkuERK1zOQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprh gtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtohepphgvfhhf sehpvghffhdrnhgvthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrd gtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:30 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 51b8ab95 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:11:53 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:29 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 01/10] reftable/blocksource: drop malloc block source 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: The reftable blocksource provides a generic interface to read blocks via different sources, e.g. from disk or from memory. One of the block sources is the malloc block source, which can in theory read data from memory. We nowadays also have a strbuf block source though, which provides essentially the same functionality with better ergonomics. Adapt the only remaining user of the malloc block source in our tests to use the strbuf block source, instead, and remove the now-unused malloc block source. Signed-off-by: Patrick Steinhardt --- reftable/block_test.c | 3 ++- reftable/blocksource.c | 20 -------------------- reftable/blocksource.h | 2 -- 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/reftable/block_test.c b/reftable/block_test.c index 90aecd5a7c6..de8f426a429 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -34,11 +34,12 @@ static void test_block_read_write(void) struct block_reader br = { 0 }; struct block_iter it = BLOCK_ITER_INIT; int j = 0; + struct strbuf block_data = STRBUF_INIT; struct strbuf want = STRBUF_INIT; REFTABLE_CALLOC_ARRAY(block.data, block_size); block.len = block_size; - block.source = malloc_block_source(); + block_source_from_strbuf(&block.source, &block_data); block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size, header_off, hash_size(GIT_SHA1_FORMAT_ID)); diff --git a/reftable/blocksource.c b/reftable/blocksource.c index eeed254ba9c..1774853011d 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -55,26 +55,6 @@ void block_source_from_strbuf(struct reftable_block_source *bs, bs->arg = buf; } -static void malloc_return_block(void *b, struct reftable_block *dest) -{ - if (dest->len) - memset(dest->data, 0xff, dest->len); - reftable_free(dest->data); -} - -static struct reftable_block_source_vtable malloc_vtable = { - .return_block = &malloc_return_block, -}; - -static struct reftable_block_source malloc_block_source_instance = { - .ops = &malloc_vtable, -}; - -struct reftable_block_source malloc_block_source(void) -{ - return malloc_block_source_instance; -} - struct file_block_source { uint64_t size; unsigned char *data; diff --git a/reftable/blocksource.h b/reftable/blocksource.h index 072e2727ad2..659a27b4063 100644 --- a/reftable/blocksource.h +++ b/reftable/blocksource.h @@ -17,6 +17,4 @@ struct reftable_block_source; void block_source_from_strbuf(struct reftable_block_source *bs, struct strbuf *buf); -struct reftable_block_source malloc_block_source(void); - #endif From patchwork Fri Aug 23 14:12:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775280 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 62FB8189521 for ; Fri, 23 Aug 2024 14:12:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422357; cv=none; b=VrGayF2MPSbQSAZovYg4g4k+DfpV2A9nquZUHWpn91ueUUltVjdH5k1JAyucDpaVMGwR0VxmRh5I+fZDb2vrOk5OsLbmRmM9nv6XgIgv8gQx6pnUaSB0OdpRazmffEU3PqRk1fZ/NgmI+zSryR2aVYvI1gaP53JrV7l0yTyhiXY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422357; c=relaxed/simple; bh=ldtUrMemno6DYaGxJNd1d4lTu6bzZSdyz/i2l1NdJbE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hlsapiFfOhwe85EDZA5WhF91/1agwL1QQEbfrQmJu3OfUkV3Hi42A/SOWGCAe39lgUdbnCtd+YJZbVkOHnwErlleQGAuxduAWj8O46FaOpQTLNQ7aBSA54Ida6CftqBNs4VM5fU4A8LdCNfoOWXG5AKfr9KGtNvnoj6izhWEiQs= 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=TvoAEGgT; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=gYGY48Rb; arc=none smtp.client-ip=103.168.172.145 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="TvoAEGgT"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="gYGY48Rb" Received: from phl-compute-06.internal (phl-compute-06.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 7659E138FFCB; Fri, 23 Aug 2024 10:12:34 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Fri, 23 Aug 2024 10:12:34 -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=fm1; t=1724422354; x=1724508754; bh=HFfoI+cj3M N+A52EBzM78o6HUx3a1PkzzTk021v3CHk=; b=TvoAEGgTGgnQI3aBTvvMYQ1Pbw 5fWFakVNxAPL6KpD8STOzq4inLE3yqfJxehlAF4eRKZA9htMJ2V8/+i6ZhsURXj8 EQaYznJGd6iXUB85y6vY3WmZPA3dqzNpWOq+jfwN+bHOmrdPxS0WBz9CScEin4aU oeEyNfWm0Hhb8Co1eH5jA0V9qeAFPg23KRWBS58VuPh+HcamyDlF5YkCQ/rqaazU 4dTVjNePOl2HA3mKK3xjm/viDPNzO45JMjdPzKT7jFzEY9s8ByWbnOudtw/yL7bf ODSFprIdYviuXEyY/BpBKUVVKgGQrdE4lqYKMzIjv2ys/tkhsXvG5mKtP/ZA== 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= fm1; t=1724422354; x=1724508754; bh=HFfoI+cj3MN+A52EBzM78o6HUx3a 1PkzzTk021v3CHk=; b=gYGY48Rbq/3wMZhB5TNq9GOj7OPbu/IxgTi0y3rmQzNX NP+r1PGP1nmKZELTgvxtN6zzHYipGLPd+aWF00XPcvu/37lRg9a/6tewYhwDq1bI gYqkP3755S0G1JfOnIgGsOfqirJCZaOsLD3D7VeL6oH+0OvXt/QZhoEK136oA5Rp OsqJj2kr4GoxdKD1JhJpAewJc63cIl6wfMXU/9HhuBsdtQBjCVS4X2glguVpESqY aTAn/E4FREvCju1Wr7vioHGxCSR2M1xjrqxNR4takikC0KeN5Z4ZJNhUtsItiUCs VOGALQ7Xf4GrkJuHwkxTRk8/RGmNwf9mEwaub7IxTg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprh gtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohep phgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrd gtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:33 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 9204415d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:11:56 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:32 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 02/10] reftable/stack: inline `stack_compact_range_stats()` Message-ID: <3c0cf2bf46f4106d7c0b30c863946262639b2351.1724420744.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 only difference between `stack_compact_range_stats()` and `stack_compact_range()` is that the former updates stats on failure, whereas the latter doesn't. There are no callers anymore that do not want their stats updated though, making the indirection unnecessary. Inline the stat updates into `stack_compact_range()`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index d3a95d2f1d7..891ea971b72 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1328,17 +1328,9 @@ static int stack_compact_range(struct reftable_stack *st, strbuf_release(&table_name); free_names(names); - return err; -} - -static int stack_compact_range_stats(struct reftable_stack *st, - size_t first, size_t last, - struct reftable_log_expiry_config *config, - unsigned int flags) -{ - int err = stack_compact_range(st, first, last, config, flags); if (err == REFTABLE_LOCK_ERROR) st->stats.failures++; + return err; } @@ -1346,7 +1338,7 @@ int reftable_stack_compact_all(struct reftable_stack *st, struct reftable_log_expiry_config *config) { size_t last = st->merged->readers_len ? st->merged->readers_len - 1 : 0; - return stack_compact_range_stats(st, 0, last, config, 0); + return stack_compact_range(st, 0, last, config, 0); } static int segment_size(struct segment *s) @@ -1452,8 +1444,8 @@ int reftable_stack_auto_compact(struct reftable_stack *st) st->opts.auto_compaction_factor); reftable_free(sizes); if (segment_size(&seg) > 0) - return stack_compact_range_stats(st, seg.start, seg.end - 1, - NULL, STACK_COMPACT_RANGE_BEST_EFFORT); + return stack_compact_range(st, seg.start, seg.end - 1, + NULL, STACK_COMPACT_RANGE_BEST_EFFORT); return 0; } From patchwork Fri Aug 23 14:12:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775281 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 C20F9192B86 for ; Fri, 23 Aug 2024 14:12:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422360; cv=none; b=o2zhJ8YhcwtiI4eS0mTEWi9y8ZDp/qBYbKglQ1af2XIrSJrEGZT0yz2rqFomZaOA9gCuaqBG4jb8gpm8BU3j7kH+Hm9WWXAaMy0didSqgoGS9a+PqN7zqKAioWSPhcHCEIq5FGhPgadwdTedkZSFE6AdW3bHInodEKJMXjQHtfA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422360; c=relaxed/simple; bh=KhJQfK4t56PUtkaWwpXHdxrCHPHzsDDM9BE9VZc/cSI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ALJAg/UGbq1dvn9NBj+LE+/kJEMpfSDYKSsj+7UcGkbqm4dv9ZwFo5U6So6abB56pHiiAtEPqgfYfHZ1ipXyhQ8DE/vL8aiNkaMEDxbF+zNXLgfvv/T6DRKFUfEErAuJun6jPq3bi5gft4KStCIN1bjlUvoFZL54Kz0U/Jy9BOE= 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=MZQaVbDV; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=FhRl80nI; arc=none smtp.client-ip=103.168.172.145 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="MZQaVbDV"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="FhRl80nI" Received: from phl-compute-01.internal (phl-compute-01.nyi.internal [10.202.2.41]) by mailfout.nyi.internal (Postfix) with ESMTP id D10F8138C92C; Fri, 23 Aug 2024 10:12:37 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Fri, 23 Aug 2024 10:12:37 -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=fm1; t=1724422357; x=1724508757; bh=IY/5AUnp4X k5Pc9iVRmEHZ0teDlUv0wUWehX3wlTuNU=; b=MZQaVbDVi/gYaEOQFDteFtHS54 moMBi40vF+kk8/9Z19SreKlSaomSTIlVqGkK7/9rV/eCEIrBPGGNywk2+f3aWrLc pHuruvjrb+UIpeAAPO+gmKO0EDJZKexkqUlClc0Y75Q/LjR1id67ruvkVeT+cA5g d8gEU4iNsmiJZIdJE9Uverr51leTNgiKRIBpOFpK7FGSOb9nFSffals0bfm9BzXf EgyL1kffxnBTQMs01BnfGe88FUauwtYOm8HCLMtbMtQA5CNqzCbWcZE+6RYeF8EX cXe3KsxuhpkuzuBxJwKB8C4SSqfmREqVRIvP5VtqhUDKxhPEz0WnYj366C0g== 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= fm1; t=1724422357; x=1724508757; bh=IY/5AUnp4Xk5Pc9iVRmEHZ0teDlU v0wUWehX3wlTuNU=; b=FhRl80nI2INXuSC7wNjajKQrRcEgTs+diBAUGJXkIsTZ nZ4OTyQXb5SuXwXtTkQwefG1oCJdDbJFHFqMj1VQtEQzFqVRPjpcXUdMGz6MH9Yy 9fB0pshlldSTq5HPsviXYj5vI5bFl9xR6QP7HTlW+WzhQpb0615B6sMMdlcgfGRX KR388bxUDfd8Kzgk3qjrxKr+uUpYkuWNLV1PcYKReKWhpN++X2P9uDk3TxuMOHL9 lZfIQ9nJCjGQIAqyeJa932NjuRKkkiJyyE64/vo1QgSxjG6Jyf64k6wHjK8rVDAP JeZ56F7syvUpkeEmQE+4bkfHSPdECPTFtbgXkcpYCg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh dprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohep phgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrd gtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:36 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 82db40ff (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:11:59 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:34 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 03/10] reftable/reader: rename `reftable_new_reader()` 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: Rename the `reftable_new_reader()` function to `reftable_reader_new()` to match our coding guidelines. Signed-off-by: Patrick Steinhardt --- reftable/reader.c | 4 ++-- reftable/readwrite_test.c | 6 +++--- reftable/reftable-reader.h | 4 ++-- reftable/stack.c | 4 ++-- t/helper/test-reftable.c | 2 +- t/unit-tests/t-reftable-merged.c | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/reftable/reader.c b/reftable/reader.c index 082cf00b606..ea4fdea90b6 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -637,7 +637,7 @@ void reader_close(struct reftable_reader *r) FREE_AND_NULL(r->name); } -int reftable_new_reader(struct reftable_reader **p, +int reftable_reader_new(struct reftable_reader **p, struct reftable_block_source *src, char const *name) { struct reftable_reader *rd = reftable_calloc(1, sizeof(*rd)); @@ -786,7 +786,7 @@ int reftable_reader_print_blocks(const char *tablename) if (err < 0) goto done; - err = reftable_new_reader(&r, &src, tablename); + err = reftable_reader_new(&r, &src, tablename); if (err < 0) goto done; diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index f411abfe9cb..101cdb5cd66 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -646,7 +646,7 @@ static void test_write_empty_table(void) block_source_from_strbuf(&source, &buf); - err = reftable_new_reader(&rd, &source, "filename"); + err = reftable_reader_new(&rd, &source, "filename"); EXPECT_ERR(err); reftable_reader_init_ref_iterator(rd, &it); @@ -850,7 +850,7 @@ static void test_write_multiple_indices(void) EXPECT(stats->log_stats.index_offset > 0); block_source_from_strbuf(&source, &writer_buf); - err = reftable_new_reader(&reader, &source, "filename"); + err = reftable_reader_new(&reader, &source, "filename"); EXPECT_ERR(err); /* @@ -907,7 +907,7 @@ static void test_write_multi_level_index(void) EXPECT(stats->ref_stats.max_index_level == 2); block_source_from_strbuf(&source, &writer_buf); - err = reftable_new_reader(&reader, &source, "filename"); + err = reftable_reader_new(&reader, &source, "filename"); EXPECT_ERR(err); /* diff --git a/reftable/reftable-reader.h b/reftable/reftable-reader.h index 69621c5b0fc..8a05c846858 100644 --- a/reftable/reftable-reader.h +++ b/reftable/reftable-reader.h @@ -23,14 +23,14 @@ /* The reader struct is a handle to an open reftable file. */ struct reftable_reader; -/* reftable_new_reader opens a reftable for reading. If successful, +/* reftable_reader_new opens a reftable for reading. If successful, * returns 0 code and sets pp. The name is used for creating a * stack. Typically, it is the basename of the file. The block source * `src` is owned by the reader, and is closed on calling * reftable_reader_destroy(). On error, the block source `src` is * closed as well. */ -int reftable_new_reader(struct reftable_reader **pp, +int reftable_reader_new(struct reftable_reader **pp, struct reftable_block_source *src, const char *name); /* Initialize a reftable iterator for reading refs. */ diff --git a/reftable/stack.c b/reftable/stack.c index 891ea971b72..c72435b0596 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -258,7 +258,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, if (err < 0) goto done; - err = reftable_new_reader(&rd, &src, name); + err = reftable_reader_new(&rd, &src, name); if (err < 0) goto done; } @@ -1532,7 +1532,7 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max, if (err < 0) goto done; - err = reftable_new_reader(&rd, &src, name); + err = reftable_reader_new(&rd, &src, name); if (err < 0) goto done; diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index c1942156b50..87c2f42aaed 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -139,7 +139,7 @@ static int dump_reftable(const char *tablename) if (err < 0) goto done; - err = reftable_new_reader(&r, &src, tablename); + err = reftable_reader_new(&r, &src, tablename); if (err < 0) goto done; diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 93345c6c8be..8f51aca1b62 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -102,7 +102,7 @@ merged_table_from_records(struct reftable_ref_record **refs, write_test_table(&buf[i], refs[i], sizes[i]); block_source_from_strbuf(&(*source)[i], &buf[i]); - err = reftable_new_reader(&(*readers)[i], &(*source)[i], + err = reftable_reader_new(&(*readers)[i], &(*source)[i], "name"); check(!err); } @@ -277,7 +277,7 @@ merged_table_from_log_records(struct reftable_log_record **logs, write_test_log_table(&buf[i], logs[i], sizes[i], i + 1); block_source_from_strbuf(&(*source)[i], &buf[i]); - err = reftable_new_reader(&(*readers)[i], &(*source)[i], + err = reftable_reader_new(&(*readers)[i], &(*source)[i], "name"); check(!err); } @@ -426,7 +426,7 @@ static void t_default_write_opts(void) block_source_from_strbuf(&source, &buf); - err = reftable_new_reader(&rd, &source, "filename"); + err = reftable_reader_new(&rd, &source, "filename"); check(!err); hash_id = reftable_reader_hash_id(rd); From patchwork Fri Aug 23 14:12: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: 13775282 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 5E08C18BB94 for ; Fri, 23 Aug 2024 14:12:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422366; cv=none; b=ebw3Y/ykJ/1QIZhan/kwTXLZIEfRNcUfiEbU5Ezjsb5yhGp6/XWN8A3wJeLurXEeQiPt93b4pGWHVhNPWFQOK0r7rUibMhuyqSUi2f+6iH7fM98xRnwkD8FIQTlhHmQMlQ8aDzSD0C6TJUs+qYfLqgQSNilpqj+zu9YdhBMYKY8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422366; c=relaxed/simple; bh=nktoUlX1I5/mhgC7giDFsmwkfdc2tGUfBldMysdUMjU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=T9shrUJWnYRE+JMMvRYDQFSYamaZBGXzMiRUE0LhlZk2w5snIxQedZA8NNbvnr8aSuwWS63bvOAoHmi69Sm0uDaNGTtj2OrDuWHduP1yChM+hRZaA6Ru6WL5H+Acq1sKgxY5odOHhMiwD30B0+smzYJyPBZCnCIpRNnxiixzssg= 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=m9eMONNI; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=hLknHfGd; arc=none smtp.client-ip=103.168.172.156 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="m9eMONNI"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hLknHfGd" Received: from phl-compute-06.internal (phl-compute-06.nyi.internal [10.202.2.46]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 32AB0114875E; Fri, 23 Aug 2024 10:12:43 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Fri, 23 Aug 2024 10:12:43 -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=fm1; t=1724422363; x=1724508763; bh=Wd2a0r7EKv 9Bk5KCWTcXYDg7eiRLDUaKIaLs9Oe0i9w=; b=m9eMONNIofSREqscJSzjvOVGYM vmf1umCF9udgquTmyTnQOb2hLOAk5oQHFKNPyZ1ARPx4zwKbCqd8TqpiswjgQdta K/L3SviGFQ6IbdlQg9eAqzU9kgY8PP+dr1FEc44sqjk4YNBrE/0urw8lyX67KMaH sNm5INkDf8ZimPPYW6T4FSZQgMQRhsccmMizCjtLCLtP320YE5v3l3hgH8Lfes3h hMFbgFJXhQl+mGAlv3+canQIZlnCROENAnzX/HGclG5uq+/MevP5lQLkhP5ulN0E /cl2A9uoDcHJsFfQZ+1ljH7afhI6kpLHYgzXbCvZ7uHILWikeSG5BIIsgBFA== 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= fm1; t=1724422363; x=1724508763; bh=Wd2a0r7EKv9Bk5KCWTcXYDg7eiRL DUaKIaLs9Oe0i9w=; b=hLknHfGd44ogE5OUF/H8cef60OyRyJMn1hOxSpcurDN/ EFIc2pW4xSR5uioHsXPNxL0eFk7R4/4JnMCpXrz4IGDm0NOafh33xTqrVY+77QXi KN8lOc3cqncDnT57m3dfw9pDD4KH86yaXYUo2KQQMeGnOxwbSf8GVn3vOWoK1BtE 865Hq5G+2yStvKH2tlDiNcCS9XJBXzI/YskKgZCgSVESfYxeq7vXeidgF401octV qMa+GBjnlDF+q+YRYDXxUj19WTYifoFc0Gw7yIcvRLQeGywhwfHvIticcCx0SFK7 /7eoHLVs1Hb4hHlNZiSlFZZahHCVrQjZq4DD8BbHMQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnheptdetudffkedtffelvdeghfeuvdekieeuheeiudfgjedu ffegheffleelkeetjedtnecuffhomhgrihhnpehosghjpghofhhfshgvthhsrdhishenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhk shdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhmthhpohhuthdprhgtphhtth hopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepphgvfhhfsehp vghffhdrnhgvthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtg hpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:41 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c7eff9d3 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:05 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:37 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 04/10] reftable/reader: inline `init_reader()` Message-ID: <3b6670975011ea5126c6584be48500b9fc241ab0.1724420744.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: Most users use an allocated version of the `reftable_reader`, except for some tests. We are about to convert the reader to become refcounted though, and providing the ability to keep a reader on the stack makes this conversion harder than necessary. Update the tests to use `reftable_reader_new()` instead to prepare for this change. Signed-off-by: Patrick Steinhardt --- reftable/reader.c | 122 +++++++++++++++++++------------------- reftable/reader.h | 2 - reftable/readwrite_test.c | 73 ++++++++++++----------- 3 files changed, 98 insertions(+), 99 deletions(-) diff --git a/reftable/reader.c b/reftable/reader.c index ea4fdea90b6..9239679ad95 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -162,58 +162,6 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer, return err; } -int init_reader(struct reftable_reader *r, struct reftable_block_source *source, - const char *name) -{ - struct reftable_block footer = { NULL }; - struct reftable_block header = { NULL }; - int err = 0; - uint64_t file_size = block_source_size(source); - - /* Need +1 to read type of first block. */ - uint32_t read_size = header_size(2) + 1; /* read v2 because it's larger. */ - memset(r, 0, sizeof(struct reftable_reader)); - - if (read_size > file_size) { - err = REFTABLE_FORMAT_ERROR; - goto done; - } - - err = block_source_read_block(source, &header, 0, read_size); - if (err != read_size) { - err = REFTABLE_IO_ERROR; - goto done; - } - - if (memcmp(header.data, "REFT", 4)) { - err = REFTABLE_FORMAT_ERROR; - goto done; - } - r->version = header.data[4]; - if (r->version != 1 && r->version != 2) { - err = REFTABLE_FORMAT_ERROR; - goto done; - } - - r->size = file_size - footer_size(r->version); - r->source = *source; - r->name = xstrdup(name); - r->hash_id = 0; - - err = block_source_read_block(source, &footer, r->size, - footer_size(r->version)); - if (err != footer_size(r->version)) { - err = REFTABLE_IO_ERROR; - goto done; - } - - err = parse_footer(r, footer.data, header.data); -done: - reftable_block_done(&footer); - reftable_block_done(&header); - return err; -} - struct table_iter { struct reftable_reader *r; uint8_t typ; @@ -637,16 +585,68 @@ void reader_close(struct reftable_reader *r) FREE_AND_NULL(r->name); } -int reftable_reader_new(struct reftable_reader **p, - struct reftable_block_source *src, char const *name) +int reftable_reader_new(struct reftable_reader **out, + struct reftable_block_source *source, char const *name) { - struct reftable_reader *rd = reftable_calloc(1, sizeof(*rd)); - int err = init_reader(rd, src, name); - if (err == 0) { - *p = rd; - } else { - block_source_close(src); - reftable_free(rd); + struct reftable_block footer = { 0 }; + struct reftable_block header = { 0 }; + struct reftable_reader *r; + uint64_t file_size = block_source_size(source); + uint32_t read_size; + int err; + + REFTABLE_CALLOC_ARRAY(r, 1); + + /* + * We need one extra byte to read the type of first block. We also + * pretend to always be reading v2 of the format because it is larger. + */ + read_size = header_size(2) + 1; + if (read_size > file_size) { + err = REFTABLE_FORMAT_ERROR; + goto done; + } + + err = block_source_read_block(source, &header, 0, read_size); + if (err != read_size) { + err = REFTABLE_IO_ERROR; + goto done; + } + + if (memcmp(header.data, "REFT", 4)) { + err = REFTABLE_FORMAT_ERROR; + goto done; + } + r->version = header.data[4]; + if (r->version != 1 && r->version != 2) { + err = REFTABLE_FORMAT_ERROR; + goto done; + } + + r->size = file_size - footer_size(r->version); + r->source = *source; + r->name = xstrdup(name); + r->hash_id = 0; + + err = block_source_read_block(source, &footer, r->size, + footer_size(r->version)); + if (err != footer_size(r->version)) { + err = REFTABLE_IO_ERROR; + goto done; + } + + err = parse_footer(r, footer.data, header.data); + if (err) + goto done; + + *out = r; + +done: + reftable_block_done(&footer); + reftable_block_done(&header); + if (err) { + reftable_free(r); + block_source_close(source); } return err; } diff --git a/reftable/reader.h b/reftable/reader.h index a2c204d523c..762cd6de667 100644 --- a/reftable/reader.h +++ b/reftable/reader.h @@ -52,8 +52,6 @@ struct reftable_reader { struct reftable_reader_offsets log_offsets; }; -int init_reader(struct reftable_reader *r, struct reftable_block_source *source, - const char *name); void reader_close(struct reftable_reader *r); const char *reader_name(struct reftable_reader *r); diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 101cdb5cd66..2f2ff787b26 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -195,7 +195,7 @@ static void test_log_write_read(void) struct reftable_log_record log = { NULL }; int n; struct reftable_iterator it = { NULL }; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; struct reftable_block_source source = { NULL }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = @@ -236,10 +236,10 @@ static void test_log_write_read(void) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.log"); + err = reftable_reader_new(&reader, &source, "file.log"); EXPECT_ERR(err); - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, names[N - 1]); EXPECT_ERR(err); @@ -254,7 +254,7 @@ static void test_log_write_read(void) reftable_iterator_destroy(&it); reftable_ref_record_release(&ref); - reftable_reader_init_log_iterator(&rd, &it); + reftable_reader_init_log_iterator(reader, &it); err = reftable_iterator_seek_log(&it, ""); EXPECT_ERR(err); @@ -279,7 +279,7 @@ static void test_log_write_read(void) /* cleanup. */ strbuf_release(&buf); free_names(names); - reader_close(&rd); + reftable_reader_free(reader); } static void test_log_zlib_corruption(void) @@ -288,7 +288,7 @@ static void test_log_zlib_corruption(void) .block_size = 256, }; struct reftable_iterator it = { 0 }; - struct reftable_reader rd = { 0 }; + struct reftable_reader *reader; struct reftable_block_source source = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_writer *w = @@ -331,18 +331,18 @@ static void test_log_zlib_corruption(void) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.log"); + err = reftable_reader_new(&reader, &source, "file.log"); EXPECT_ERR(err); - reftable_reader_init_log_iterator(&rd, &it); + reftable_reader_init_log_iterator(reader, &it); err = reftable_iterator_seek_log(&it, "refname"); EXPECT(err == REFTABLE_ZLIB_ERROR); reftable_iterator_destroy(&it); /* cleanup. */ + reftable_reader_free(reader); strbuf_release(&buf); - reader_close(&rd); } static void test_table_read_write_sequential(void) @@ -352,7 +352,7 @@ static void test_table_read_write_sequential(void) int N = 50; struct reftable_iterator it = { NULL }; struct reftable_block_source source = { NULL }; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; int err = 0; int j = 0; @@ -360,10 +360,10 @@ static void test_table_read_write_sequential(void) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.ref"); + err = reftable_reader_new(&reader, &source, "file.ref"); EXPECT_ERR(err); - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, ""); EXPECT_ERR(err); @@ -381,11 +381,11 @@ static void test_table_read_write_sequential(void) reftable_ref_record_release(&ref); } EXPECT(j == N); + reftable_iterator_destroy(&it); + reftable_reader_free(reader); strbuf_release(&buf); free_names(names); - - reader_close(&rd); } static void test_table_write_small_table(void) @@ -404,7 +404,7 @@ static void test_table_read_api(void) char **names; struct strbuf buf = STRBUF_INIT; int N = 50; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; struct reftable_block_source source = { NULL }; int err; int i; @@ -415,10 +415,10 @@ static void test_table_read_api(void) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.ref"); + err = reftable_reader_new(&reader, &source, "file.ref"); EXPECT_ERR(err); - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, names[0]); EXPECT_ERR(err); @@ -431,7 +431,7 @@ static void test_table_read_api(void) } reftable_iterator_destroy(&it); reftable_free(names); - reader_close(&rd); + reftable_reader_free(reader); strbuf_release(&buf); } @@ -440,7 +440,7 @@ static void test_table_read_write_seek(int index, int hash_id) char **names; struct strbuf buf = STRBUF_INIT; int N = 50; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; struct reftable_block_source source = { NULL }; int err; int i = 0; @@ -453,18 +453,18 @@ static void test_table_read_write_seek(int index, int hash_id) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.ref"); + err = reftable_reader_new(&reader, &source, "file.ref"); EXPECT_ERR(err); - EXPECT(hash_id == reftable_reader_hash_id(&rd)); + EXPECT(hash_id == reftable_reader_hash_id(reader)); if (!index) { - rd.ref_offsets.index_offset = 0; + reader->ref_offsets.index_offset = 0; } else { - EXPECT(rd.ref_offsets.index_offset > 0); + EXPECT(reader->ref_offsets.index_offset > 0); } for (i = 1; i < N; i++) { - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, names[i]); EXPECT_ERR(err); err = reftable_iterator_next_ref(&it, &ref); @@ -480,7 +480,7 @@ static void test_table_read_write_seek(int index, int hash_id) strbuf_addstr(&pastLast, names[N - 1]); strbuf_addstr(&pastLast, "/"); - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, pastLast.buf); if (err == 0) { struct reftable_ref_record ref = { NULL }; @@ -498,7 +498,7 @@ static void test_table_read_write_seek(int index, int hash_id) reftable_free(names[i]); } reftable_free(names); - reader_close(&rd); + reftable_reader_free(reader); } static void test_table_read_write_seek_linear(void) @@ -530,7 +530,7 @@ static void test_table_refs_for(int indexed) int i = 0; int n; int err; - struct reftable_reader rd; + struct reftable_reader *reader; struct reftable_block_source source = { NULL }; struct strbuf buf = STRBUF_INIT; @@ -579,18 +579,18 @@ static void test_table_refs_for(int indexed) block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.ref"); + err = reftable_reader_new(&reader, &source, "file.ref"); EXPECT_ERR(err); if (!indexed) { - rd.obj_offsets.is_present = 0; + reader->obj_offsets.is_present = 0; } - reftable_reader_init_ref_iterator(&rd, &it); + reftable_reader_init_ref_iterator(reader, &it); err = reftable_iterator_seek_ref(&it, ""); EXPECT_ERR(err); reftable_iterator_destroy(&it); - err = reftable_reader_refs_for(&rd, &it, want_hash); + err = reftable_reader_refs_for(reader, &it, want_hash); EXPECT_ERR(err); j = 0; @@ -611,7 +611,7 @@ static void test_table_refs_for(int indexed) strbuf_release(&buf); free_names(want_names); reftable_iterator_destroy(&it); - reader_close(&rd); + reftable_reader_free(reader); } static void test_table_refs_for_no_index(void) @@ -928,11 +928,11 @@ static void test_corrupt_table_empty(void) { struct strbuf buf = STRBUF_INIT; struct reftable_block_source source = { NULL }; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; int err; block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.log"); + err = reftable_reader_new(&reader, &source, "file.log"); EXPECT(err == REFTABLE_FORMAT_ERROR); } @@ -941,13 +941,14 @@ static void test_corrupt_table(void) uint8_t zeros[1024] = { 0 }; struct strbuf buf = STRBUF_INIT; struct reftable_block_source source = { NULL }; - struct reftable_reader rd = { NULL }; + struct reftable_reader *reader; int err; strbuf_add(&buf, zeros, sizeof(zeros)); block_source_from_strbuf(&source, &buf); - err = init_reader(&rd, &source, "file.log"); + err = reftable_reader_new(&reader, &source, "file.log"); EXPECT(err == REFTABLE_FORMAT_ERROR); + strbuf_release(&buf); } From patchwork Fri Aug 23 14:12:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775283 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 8F3A6193079 for ; Fri, 23 Aug 2024 14:12:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422369; cv=none; b=pGoD96q3MMTpOfXdzurr6kbPDbX5J7ZSDiQ5rQsUpaEy0yUgK3fTJ9V+aHGmkGffZxb4CQJzrcvwiO4WbdxeyHn7OHvAgyvXQsrQSWxJm0GJp1bPIZuWKVwM4YqhVLFwwjlZOCpM+bxezSj5YNLj8LvvfM8+NivpsOtBaJwKNhk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422369; c=relaxed/simple; bh=ZgSunjuh6ggOd8p/VvvntkSSOQweJDtzHpRiLnx5aZU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=W4FYtFj3L2RbPDrbdUu2nAazWk+JRgQez84y/cycPfZKf3xbgOVKIk5Ijp0mwl7y2+LEl75oiIp7mNSBQYQVLrweD0BVmuG4C221E5YYLxFU4pSNmEE25edCXINmKy954Z0uN12AGe8Oj07AushNxeceYPwEIbswB4lF5OA7S1s= 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=SZW+Zsie; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=RUkJX99E; arc=none smtp.client-ip=103.168.172.145 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="SZW+Zsie"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="RUkJX99E" Received: from phl-compute-02.internal (phl-compute-02.nyi.internal [10.202.2.42]) by mailfout.nyi.internal (Postfix) with ESMTP id 92874138FFCD; Fri, 23 Aug 2024 10:12:46 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-02.internal (MEProxy); Fri, 23 Aug 2024 10:12:46 -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=fm1; t=1724422366; x=1724508766; bh=Q+FierekXH iyCpu0QO+M9TUVyqp6BxOqI9P3ZH60sAk=; b=SZW+ZsiehrYM5EnrgRfF/FiQo3 wW00AJIfdzgpu9E9r94SmAdwsazROYw4MRta/MZ71+DrJU2cqoFNbtPverkgJpDV HcP0hc5EfS8846hIE+7ckYpNpXt3PB5E+IOlMvkTKiUM6lMa3s6+XUEPo01e5ojd w8lNNXbZVyNHk+6kxL8enWjTpocOMzfODc8moUk+496Qpha68sWHCUC+ke5RlXzI 7pT5SCYo+fLGfnEfwPIMjWzaHRyhw3JjP/7YpewVScNDyvY9aVdZHvD/tSwV8gGM JsXVirs+Yp/DBxAUFyhJhYydwZyPSjcMxnnfDe2KiFf84/pRrkIBIH83bHmg== 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= fm1; t=1724422366; x=1724508766; bh=Q+FierekXHiyCpu0QO+M9TUVyqp6 BxOqI9P3ZH60sAk=; b=RUkJX99Evi2Hhe4ZfHx7QrN5q1/ZwFnVDqSMXA5dSSjN UCTXtXQax2O8kn1jBKUxydTkjDmm0WN6ttmVbotkZf1GOqQLvUylaIm1FLQvnBUh BJshUTluCbQBPDzY5ZZWGyVYEBwq15yEfCCpgUy+vmRJuaPCOJR1N7mCip0z6yVo JrmGpJgRXsRh7AP9/se30ESjjicIu0HmX8uVa8Xqb/vIPj4AskNaF+43egrAiWvf dXOspNbEcVQ/o5Kga4dANOiKfFEo07Bf9Ec2kUYAUh4rljbTG7KhcW7/lAchpvzS n5R6Ja8XbTJfmp+xhJPgNvhsjrQucV6x/dcNE6ryLA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh dprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtohepphgv fhhfsehpvghffhdrnhgvthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrd horhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:45 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 892fe8b5 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:07 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:43 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 05/10] reftable/reader: inline `reader_close()` 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: Same as with the preceding commit, we also provide a `reader_close()` function that allows the caller to close a reader without freeing it. This is unnecessary now that all users will have an allocated version of the reader. Inline it into `reftable_reader_free()`. Signed-off-by: Patrick Steinhardt --- reftable/reader.c | 9 ++------- reftable/reader.h | 1 - reftable/stack.c | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/reftable/reader.c b/reftable/reader.c index 9239679ad95..037417fcf63 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -579,12 +579,6 @@ void reftable_reader_init_log_iterator(struct reftable_reader *r, reader_init_iter(r, it, BLOCK_TYPE_LOG); } -void reader_close(struct reftable_reader *r) -{ - block_source_close(&r->source); - FREE_AND_NULL(r->name); -} - int reftable_reader_new(struct reftable_reader **out, struct reftable_block_source *source, char const *name) { @@ -655,7 +649,8 @@ void reftable_reader_free(struct reftable_reader *r) { if (!r) return; - reader_close(r); + block_source_close(&r->source); + FREE_AND_NULL(r->name); reftable_free(r); } diff --git a/reftable/reader.h b/reftable/reader.h index 762cd6de667..88b4f3b4212 100644 --- a/reftable/reader.h +++ b/reftable/reader.h @@ -52,7 +52,6 @@ struct reftable_reader { struct reftable_reader_offsets log_offsets; }; -void reader_close(struct reftable_reader *r); const char *reader_name(struct reftable_reader *r); void reader_init_iter(struct reftable_reader *r, diff --git a/reftable/stack.c b/reftable/stack.c index c72435b0596..0ac9cdf8de1 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -290,7 +290,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st, const char *name = reader_name(cur[i]); stack_filename(&table_path, st, name); - reader_close(cur[i]); reftable_reader_free(cur[i]); /* On Windows, can only unlink after closing. */ @@ -299,10 +298,8 @@ static int reftable_stack_reload_once(struct reftable_stack *st, } done: - for (i = 0; i < new_readers_len; i++) { - reader_close(new_readers[i]); + for (i = 0; i < new_readers_len; i++) reftable_reader_free(new_readers[i]); - } reftable_free(new_readers); reftable_free(cur); strbuf_release(&table_path); From patchwork Fri Aug 23 14:12:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775284 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 C0635193089 for ; Fri, 23 Aug 2024 14:12:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422371; cv=none; b=frHg5e8seBpmfscZUkbKhe/jS/J5n3H/x8nRvJCO9i5HfN5ULP1Kv/BRRW/JTmAmTCvaOMyUqwh0vDSJN9cGoOUpsG8+K3YfcF8bnfnbiscsA68p5XYyRhO5/srtkzoH6vQDqLBlL197R5gzWyFQlKlr04a9Aua0AaVtp4FiB8s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422371; c=relaxed/simple; bh=CxCN1HxCKt5CyjmkvvUW28BYXyq71hsH0/JRus5sI/4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Q24vdjPpDIzqKr/LIciFdIavDfg60nhiyiFUTd2wL8StSCdNcVeZrTThnklaA56hanvMyLYxv4biKS6XQpKboWtX1CqgqHNt8NIDrDHnvzdtNwiOQrUlMIxtRbX9/NBtdVbFxoDKEUwA4XWah/myoeWid6m/sl9asa5F0UVtrEM= 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=JepQgtJT; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ii92zQld; arc=none smtp.client-ip=103.168.172.145 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="JepQgtJT"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ii92zQld" Received: from phl-compute-04.internal (phl-compute-04.nyi.internal [10.202.2.44]) by mailfout.nyi.internal (Postfix) with ESMTP id EA4E5138FF42; Fri, 23 Aug 2024 10:12:48 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Fri, 23 Aug 2024 10:12:48 -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=fm1; t=1724422368; x=1724508768; bh=UFC1CjraK+ CojECdNRi4i8zXe2r8WQvuWqj53WEVd4w=; b=JepQgtJT5cRMj/2iaSi2IGfs2y w3bBs6fGA/xJ6ABj2MTq08cECmydL3Pwy7XA01GrDtu2DH0qccTWfEaaR/DAjKp2 TDBMz1r2oN3yVWyx3bU+1GLSiv8JmOEC+YzDjmMw0qSo46diSJLxSeMeLqVJZUW4 6fF9pl0C6rO5I0iN8mavEofH+pZpiMEfAJdA+cGGhclkwZx5zXy6bzCdTaulbVuw nUDjJL4L8Pp3jqibRNuAXW3Zoqd4S9wqCJAK+evo8Gzt+Yze0H1kPWxwkCKw/Xsa qrzZQVtsS5HKN0p48i21a5asWhB7mawGinMmAJGE9C8sakMm/OMjKZlkQYMg== 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= fm1; t=1724422368; x=1724508768; bh=UFC1CjraK+CojECdNRi4i8zXe2r8 WQvuWqj53WEVd4w=; b=ii92zQldi/FhZ84nuS2zv0aNfHOEUraufFvDliukWJr+ 2Qm+aq+gpRUUbpFBqxAmgpPm5OeAeKVFJzDk0DCVpq6sdmDSb/igeAeFvyKJqL8G sSsMNp6q09PKDqnuJReyOMAZPDBjD3zoFEVtIka+z8Gf7QwlFAP8taWctuMc7KgL iP+FnhVop6ZHdthxezLM8vkMZvPffx5zjhu4cSTPGh1hvmvSGBDlUMOJDT21bSWS W7dwtxY27z8HfwmGkiTj41+IyJCIdCYCwZyF9oKTF+k4pXor+ppA5EPfTmSlMTjJ mqanpDUxSIQEogX1iJXZlYX5ZKs/X0v6hV2JdT2b3g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhope hgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtohepkhgrrhhthhhikhdrudek keesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrd horhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:47 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6cb25e5f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:10 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:46 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 06/10] reftable/stack: fix broken refnames in `write_n_ref_tables()` 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: The `write_n_ref_tables()` helper function writes N references in separate tables. We never reset the computed name of those references though, leading us to end up with unexpected names. Fix this by resetting the buffer. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 42044ed8a3e..de0669b7b8a 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -125,6 +125,7 @@ static void write_n_ref_tables(struct reftable_stack *st, .value_type = REFTABLE_REF_VAL1, }; + strbuf_reset(&buf); strbuf_addf(&buf, "refs/heads/branch-%04u", (unsigned) i); ref.refname = buf.buf; set_test_hash(ref.value.val1, i); From patchwork Fri Aug 23 14:12:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775285 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (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 4AD5D193406 for ; Fri, 23 Aug 2024 14:12:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422374; cv=none; b=rhKpBj3nEyC6cF5DQdwKfWiFniqvks68DtdO5yBt4ydos9DuCplhYxNYUCpKrL3Rw7RwiHyJTjFiiJwdeQGMmDlR4wS/viVVfBmU7jQ71wgYTy59OxKYNmHim1pYIskNgE7KzcrGDb3ZhZS30AD6xGY8C+VFWJK64OfpXGcsIRs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422374; c=relaxed/simple; bh=1Z9nbu+Etd56/PfWeT03l4K6QwXSgjpHxVjdAmZvdfY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Sis/EiPWediYwcVfL1fK3SHcAZLu7AEXYm2pnWOYHLTegSbX8w0o4m2shXo5VW58ye05ojbjAbQYbiLU9ug9rUnK9EYJvf1oSqooRfLBax12N3qDCNzJ6wcs+q0wm5QmaObfZrUuW327Ltfjt4+y+JCJaEVL6u52KrlZVv0srVo= 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=f1a8HBlw; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=G8iEWS8q; arc=none smtp.client-ip=103.168.172.145 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="f1a8HBlw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="G8iEWS8q" Received: from phl-compute-07.internal (phl-compute-07.nyi.internal [10.202.2.47]) by mailfout.nyi.internal (Postfix) with ESMTP id 4E5A9138D705; Fri, 23 Aug 2024 10:12:51 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Fri, 23 Aug 2024 10:12:51 -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=fm1; t=1724422371; x=1724508771; bh=rCRcWmgvbo vUtC9DP1DPRUAcqksw6t/lAnvgLzSeiRA=; b=f1a8HBlwnQUFyRHnaqihTH0DJQ Zu6Z+9BwnM0ogGWAI7uq4LYyB49xKqPK+YGGBH91YPTrHD5s6Bo2P/x1umlbHg90 kdAwB14UGa/hM7XlN75Sbx8qQiwckAtS+b64S6lokNx+CokPRIOzpJ+julU+yJce 6YDZHLQ5NjGerWJAeGOrNxvNnr5FHqdYh7ENC9p8r6zn42xcb2Cq2fcaZyM7S83r Pj52PLMemzqz8HSwgpdC/pXm1YLKf0YV5F8Hq5ck4Dl8s5HWQdYVZOUDGktRZw7z vTZG4MxjoXQDsIhBRtfh/tkIieiCsQ9SUR46pLyVYzi1kmdUsjH2BkzRnoRg== 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= fm1; t=1724422371; x=1724508771; bh=rCRcWmgvbovUtC9DP1DPRUAcqksw 6t/lAnvgLzSeiRA=; b=G8iEWS8qz5ZE8lOCLyyAoIDwWCsBgg97hJK2XWoag/js eQYx3z6STILdHa56p9LXyyyVLmlXAR0QwPaZOUoPlYXglhKUkjy24Gvd7IRITtA8 V4tnYVnw6NuJyGunsFNg0KxS/FKac8WBRAmx6BZB0TfqzFx59IA0CY0yiTbj9RG7 c4u51DO615M9CcwmxBJBiGQXqIulOHC6H+R2rWv6imrhqfsOEPl1FJ79bqLyuvIh UXwxZtQtmpBYfxyWnfeSZrJD6z8IGvmDh64nq+SosWvQE76ge/0PZTplfS7yhlPM 5dAX+D4VsdPVKhBVuoncRqS4ZIVm7kyTNCB23BFgiQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtph htthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepkhgrrhht hhhikhdrudekkeesghhmrghilhdrtghomhdprhgtphhtthhopehpvghffhesphgvfhhfrd hnvght X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:50 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ae90c031 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:13 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:48 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 07/10] reftable/reader: introduce refcounting Message-ID: <6535d1ca9dede63011333a68f885d9782bcb182c.1724420744.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: It was recently reported that concurrent reads and writes may cause the reftable backend to segfault. The root cause of this is that we do not properly keep track of reftable readers across reloads. Suppose that you have a reftable iterator and then decide to reload the stack while iterating through the iterator. When the stack has been rewritten since we have created the iterator, then we would end up discarding a subset of readers that may still be in use by the iterator. The consequence is that we now try to reference deallocated memory, which of course segfaults. One way to trigger this is in t5616, where some background maintenance jobs have been leaking from one test into another. This leads to stack traces like the following one: + git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin AddressSanitizer:DEADLYSIGNAL ================================================================= ==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp 0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0) ==657994==The signal is caused by a READ memory access. #0 0x55f23e52ddf9 in get_var_int reftable/record.c:29 #1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170 #2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194 #3 0x55f23e54e72e in block_iter_next reftable/block.c:398 #4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240 #5 0x55f23e5573dc in table_iter_next reftable/reader.c:355 #6 0x55f23e5573dc in table_iter_next reftable/reader.c:339 #7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69 #8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123 #9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172 #10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175 #11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464 #12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13 #13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452 #14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623 #15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659 #16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133 #17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432 #18 0x55f23dba7764 in run_builtin git.c:484 #19 0x55f23dba7764 in handle_builtin git.c:741 #20 0x55f23dbab61e in run_argv git.c:805 #21 0x55f23dbab61e in cmd_main git.c:1000 #22 0x55f23dba4781 in main common-main.c:64 #23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360 #25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27) While it is somewhat awkward that the maintenance processes survive tests in the first place, it is totally expected that reftables should work alright with concurrent writers. Seemingly they don't. The only underlying resource that we need to care about in this context is the reftable reader, which is responsible for reading a single table from disk. These readers get discarded immediately (unless reused) when calling `reftable_stack_reload()`, which is wrong. We can only close them once we know that there are no iterators using them anymore. Prepare for a fix by converting the reftable readers to be refcounted. Reported-by: Jeff King Signed-off-by: Patrick Steinhardt --- reftable/reader.c | 16 ++++++++++++++-- reftable/reader.h | 2 ++ reftable/readwrite_test.c | 18 +++++++++--------- reftable/reftable-reader.h | 15 ++++++++++++--- reftable/stack.c | 8 ++++---- reftable/stack_test.c | 6 ++---- t/helper/test-reftable.c | 2 +- t/unit-tests/t-reftable-merged.c | 4 ++-- 8 files changed, 46 insertions(+), 25 deletions(-) diff --git a/reftable/reader.c b/reftable/reader.c index 037417fcf63..64a0953e687 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -621,6 +621,7 @@ int reftable_reader_new(struct reftable_reader **out, r->source = *source; r->name = xstrdup(name); r->hash_id = 0; + r->refcount = 1; err = block_source_read_block(source, &footer, r->size, footer_size(r->version)); @@ -645,10 +646,21 @@ int reftable_reader_new(struct reftable_reader **out, return err; } -void reftable_reader_free(struct reftable_reader *r) +void reftable_reader_incref(struct reftable_reader *r) +{ + if (!r->refcount) + BUG("cannot increment ref counter of dead reader"); + r->refcount++; +} + +void reftable_reader_decref(struct reftable_reader *r) { if (!r) return; + if (!r->refcount) + BUG("cannot decrement ref counter of dead reader"); + if (--r->refcount) + return; block_source_close(&r->source); FREE_AND_NULL(r->name); reftable_free(r); @@ -812,7 +824,7 @@ int reftable_reader_print_blocks(const char *tablename) } done: - reftable_reader_free(r); + reftable_reader_decref(r); table_iter_close(&ti); return err; } diff --git a/reftable/reader.h b/reftable/reader.h index 88b4f3b4212..3710ee09b4c 100644 --- a/reftable/reader.h +++ b/reftable/reader.h @@ -50,6 +50,8 @@ struct reftable_reader { struct reftable_reader_offsets ref_offsets; struct reftable_reader_offsets obj_offsets; struct reftable_reader_offsets log_offsets; + + uint64_t refcount; }; const char *reader_name(struct reftable_reader *r); diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 2f2ff787b26..0494e7955a5 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -279,7 +279,7 @@ static void test_log_write_read(void) /* cleanup. */ strbuf_release(&buf); free_names(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_log_zlib_corruption(void) @@ -341,7 +341,7 @@ static void test_log_zlib_corruption(void) reftable_iterator_destroy(&it); /* cleanup. */ - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); } @@ -383,7 +383,7 @@ static void test_table_read_write_sequential(void) EXPECT(j == N); reftable_iterator_destroy(&it); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); free_names(names); } @@ -431,7 +431,7 @@ static void test_table_read_api(void) } reftable_iterator_destroy(&it); reftable_free(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); } @@ -498,7 +498,7 @@ static void test_table_read_write_seek(int index, int hash_id) reftable_free(names[i]); } reftable_free(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_table_read_write_seek_linear(void) @@ -611,7 +611,7 @@ static void test_table_refs_for(int indexed) strbuf_release(&buf); free_names(want_names); reftable_iterator_destroy(&it); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_table_refs_for_no_index(void) @@ -657,7 +657,7 @@ static void test_write_empty_table(void) EXPECT(err > 0); reftable_iterator_destroy(&it); - reftable_reader_free(rd); + reftable_reader_decref(rd); strbuf_release(&buf); } @@ -863,7 +863,7 @@ static void test_write_multiple_indices(void) reftable_iterator_destroy(&it); reftable_writer_free(writer); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&writer_buf); strbuf_release(&buf); } @@ -919,7 +919,7 @@ static void test_write_multi_level_index(void) reftable_iterator_destroy(&it); reftable_writer_free(writer); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&writer_buf); strbuf_release(&buf); } diff --git a/reftable/reftable-reader.h b/reftable/reftable-reader.h index 8a05c846858..a600452b565 100644 --- a/reftable/reftable-reader.h +++ b/reftable/reftable-reader.h @@ -33,6 +33,18 @@ struct reftable_reader; int reftable_reader_new(struct reftable_reader **pp, struct reftable_block_source *src, const char *name); +/* + * Manage the reference count of the reftable reader. A newly initialized + * reader starts with a refcount of 1 and will be deleted once the refcount has + * reached 0. + * + * This is required because readers may have longer lifetimes than the stack + * they belong to. The stack may for example be reloaded while the old tables + * are still being accessed by an iterator. + */ +void reftable_reader_incref(struct reftable_reader *reader); +void reftable_reader_decref(struct reftable_reader *reader); + /* Initialize a reftable iterator for reading refs. */ void reftable_reader_init_ref_iterator(struct reftable_reader *r, struct reftable_iterator *it); @@ -44,9 +56,6 @@ void reftable_reader_init_log_iterator(struct reftable_reader *r, /* returns the hash ID used in this table. */ uint32_t reftable_reader_hash_id(struct reftable_reader *r); -/* closes and deallocates a reader. */ -void reftable_reader_free(struct reftable_reader *); - /* return an iterator for the refs pointing to `oid`. */ int reftable_reader_refs_for(struct reftable_reader *r, struct reftable_iterator *it, uint8_t *oid); diff --git a/reftable/stack.c b/reftable/stack.c index 0ac9cdf8de1..8e85f8b4d99 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -186,7 +186,7 @@ void reftable_stack_destroy(struct reftable_stack *st) if (names && !has_name(names, name)) { stack_filename(&filename, st, name); } - reftable_reader_free(st->readers[i]); + reftable_reader_decref(st->readers[i]); if (filename.len) { /* On Windows, can only unlink after closing. */ @@ -290,7 +290,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, const char *name = reader_name(cur[i]); stack_filename(&table_path, st, name); - reftable_reader_free(cur[i]); + reftable_reader_decref(cur[i]); /* On Windows, can only unlink after closing. */ unlink(table_path.buf); @@ -299,7 +299,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, done: for (i = 0; i < new_readers_len; i++) - reftable_reader_free(new_readers[i]); + reftable_reader_decref(new_readers[i]); reftable_free(new_readers); reftable_free(cur); strbuf_release(&table_path); @@ -1534,7 +1534,7 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max, goto done; update_idx = reftable_reader_max_update_index(rd); - reftable_reader_free(rd); + reftable_reader_decref(rd); if (update_idx <= max) { unlink(table_path.buf); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index de0669b7b8a..bc3bf772749 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1036,10 +1036,8 @@ static void test_reftable_stack_compaction_concurrent(void) static void unclean_stack_close(struct reftable_stack *st) { /* break abstraction boundary to simulate unclean shutdown. */ - int i = 0; - for (; i < st->readers_len; i++) { - reftable_reader_free(st->readers[i]); - } + for (size_t i = 0; i < st->readers_len; i++) + reftable_reader_decref(st->readers[i]); st->readers_len = 0; FREE_AND_NULL(st->readers); } diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index 87c2f42aaed..f6d855a9d7f 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -152,7 +152,7 @@ static int dump_reftable(const char *tablename) done: reftable_merged_table_free(mt); - reftable_reader_free(r); + reftable_reader_decref(r); return err; } diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 8f51aca1b62..081d3c8b697 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -115,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs, static void readers_destroy(struct reftable_reader **readers, const size_t n) { for (size_t i = 0; i < n; i++) - reftable_reader_free(readers[i]); + reftable_reader_decref(readers[i]); reftable_free(readers); } @@ -437,7 +437,7 @@ static void t_default_write_opts(void) err = reftable_merged_table_new(&merged, &rd, 1, GIT_SHA1_FORMAT_ID); check(!err); - reftable_reader_free(rd); + reftable_reader_decref(rd); reftable_merged_table_free(merged); strbuf_release(&buf); } From patchwork Fri Aug 23 14:12:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775286 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 AF6A7193413 for ; Fri, 23 Aug 2024 14:12:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422377; cv=none; b=pply4za8wAApPsXIT/xcp1cntjvrd2oJZcuUj3NyIS2teGxC/WgRG/q9dTWvDewHVXUhQ79jHAJIJzZuVapo0PYb84IOz1DgsDitOPIb20p/kMxaepEaWBCvvDlKQURRV9F/Yr1aLw3L6k07fYqGtxLGqIdFqV/7SH7cVi2u2pM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422377; c=relaxed/simple; bh=sFdVDPk55EoeYZDxdtgVnP3DOVZcLom3kSAVezWQ59U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=n3oWXNaarjmwtwanNGg1i7/zpsZ04Kg2EAsKoFGT9sqvNmWZxzAOHLjhuFGvlXcSqoQZ/QhOmXRTU3h2jLQsmR7FPv3qPZ87LsceAz/xEnAY0juxtfRIw/mGrK8rqRx3ncbvngUBvXKUcUTK0EraxeDu2NUCAU4Xg7uCdwPsFs8= 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=nfdIwrFd; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=cUsVMdUe; arc=none smtp.client-ip=103.168.172.156 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="nfdIwrFd"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="cUsVMdUe" Received: from phl-compute-04.internal (phl-compute-04.nyi.internal [10.202.2.44]) by mailfhigh.nyi.internal (Postfix) with ESMTP id A7C2D1151A25; Fri, 23 Aug 2024 10:12:54 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Fri, 23 Aug 2024 10:12:54 -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=fm1; t=1724422374; x=1724508774; bh=eN7XuBg+Tg 1Nwuk2CcYL4Uv7dt+00TnZkTfhdWKT56k=; b=nfdIwrFdO2tyOFhN1MgP9hxNMZ DPdrCdzSpNhlA/Y8o8nqkCFP7OFi4A5R47C/swlYdWvbIdDisSJvaGexczNG3KQ8 4GiSl21usEOBOXoJdmhQQCT+ifvJaYYkxaGy5KoBlMmkdQe+AjtBza7AH4q3HGgb h4C99POURrgGYqO6S4ys/Hd38Oi4m08kCS0dK/ghFaJH0/b1n6OGCjtSHpB0DB2Q zyqg3m2p4fKslMF1Aet454vocW12A7kIA3NfWnJInKxg9P20fdjabYS8SLcdr6nh W6tvQ1YEJQPach32zmzHrWcGsOEV+IaqOwru3s4rp/enDTG021qz8jg1YhKw== 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= fm1; t=1724422374; x=1724508774; bh=eN7XuBg+Tg1Nwuk2CcYL4Uv7dt+0 0TnZkTfhdWKT56k=; b=cUsVMdUe6LH2+44PrXx64fJeqB+kNwn4mPrBoBH5Ck+v CC4BMkNjuz10rsiD7qvFuXy9xAgZOM0xpFY+edMtEjH07d4ea/WiUIwTviL8O4yG RPX/QV7IR30A4TcmjafZ8rvRYdoiaxAo1imv6L5zMt+idU041hpOGa63xH0IWv8a 7TKU7VBaGKhC4lAYRxGeq2g6/vbJ9KxELVLwgkJs9XAbkm2dEyPB7WOXnQmJ6Y10 agBuIzQOITJh+YlNAR0zKJvgCxFBy+AuxUR/5/t0LbPOGGI7eKggMmGk241p8shV orrGfAMY4QAVTNVo4do/bWC6ZsVHUPnK0XR7+NkoTQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh dprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpthhtohepghhithhsthgv rhesphhosghogidrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrd horhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:53 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 30781810 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:16 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:51 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 08/10] reftable/reader: keep readers alive during iteration Message-ID: <8d08c3bc515db8a3d7fc0a617a2cf1025164f472.1724420744.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 lifetime of a table iterator may survive the lifetime of a reader when the stack gets reloaded. Keep the reader from being released by increasing its refcount while the iterator is still being used. Signed-off-by: Patrick Steinhardt --- reftable/reader.c | 2 ++ reftable/stack_test.c | 50 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/reftable/reader.c b/reftable/reader.c index 64a0953e687..f8770990876 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -175,6 +175,7 @@ static int table_iter_init(struct table_iter *ti, struct reftable_reader *r) { struct block_iter bi = BLOCK_ITER_INIT; memset(ti, 0, sizeof(*ti)); + reftable_reader_incref(r); ti->r = r; ti->bi = bi; return 0; @@ -262,6 +263,7 @@ static void table_iter_close(struct table_iter *ti) { table_iter_block_done(ti); block_iter_close(&ti->bi); + reftable_reader_decref(ti->r); } static int table_iter_next_block(struct table_iter *ti) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index bc3bf772749..7fb5beb7c94 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1076,6 +1076,55 @@ static void test_reftable_stack_compaction_concurrent_clean(void) clear_dir(dir); } +static void test_reftable_stack_read_across_reload(void) +{ + struct reftable_write_options opts = { 0 }; + struct reftable_stack *st1 = NULL, *st2 = NULL; + struct reftable_ref_record rec = { 0 }; + struct reftable_iterator it = { 0 }; + char *dir = get_tmp_dir(__LINE__); + int err; + + /* Create a first stack and set up an iterator for it. */ + err = reftable_new_stack(&st1, dir, &opts); + EXPECT_ERR(err); + write_n_ref_tables(st1, 2); + EXPECT(st1->merged->readers_len == 2); + reftable_stack_init_ref_iterator(st1, &it); + err = reftable_iterator_seek_ref(&it, ""); + EXPECT_ERR(err); + + /* Set up a second stack for the same directory and compact it. */ + err = reftable_new_stack(&st2, dir, &opts); + EXPECT_ERR(err); + EXPECT(st2->merged->readers_len == 2); + err = reftable_stack_compact_all(st2, NULL); + EXPECT_ERR(err); + EXPECT(st2->merged->readers_len == 1); + + /* + * Verify that we can continue to use the old iterator even after we + * have reloaded its stack. + */ + err = reftable_stack_reload(st1); + EXPECT_ERR(err); + EXPECT(st1->merged->readers_len == 1); + err = reftable_iterator_next_ref(&it, &rec); + EXPECT_ERR(err); + EXPECT(!strcmp(rec.refname, "refs/heads/branch-0000")); + err = reftable_iterator_next_ref(&it, &rec); + EXPECT_ERR(err); + EXPECT(!strcmp(rec.refname, "refs/heads/branch-0001")); + err = reftable_iterator_next_ref(&it, &rec); + EXPECT(err > 0); + + reftable_ref_record_release(&rec); + reftable_iterator_destroy(&it); + reftable_stack_destroy(st1); + reftable_stack_destroy(st2); + clear_dir(dir); +} + int stack_test_main(int argc, const char *argv[]) { RUN_TEST(test_empty_add); @@ -1098,6 +1147,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_auto_compaction_fails_gracefully); RUN_TEST(test_reftable_stack_update_index_check); RUN_TEST(test_reftable_stack_uptodate); + RUN_TEST(test_reftable_stack_read_across_reload); RUN_TEST(test_suggest_compaction_segment); RUN_TEST(test_suggest_compaction_segment_nothing); return 0; From patchwork Fri Aug 23 14:12:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775287 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 9ED3C19341E for ; Fri, 23 Aug 2024 14:12:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422380; cv=none; b=oSmfRgzxIeFcZjGwtDrUjcXxsEeNoT+QUsjQREn606LuPAoqhg/OXDST9SEh33mVPY1Za1pKDnufd7IwMqFBSSDormYd7zMMrnm89YgxLR79wyqjtR4FyWHd1kkmnGez68Zy4Ocs0axzWCv14Gkm9HTOPeXclBWPHIeyHMvTe1U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422380; c=relaxed/simple; bh=GardCaz1qqT1M8dmGQdW9t3ITLT/nprLJBrpkEANDDE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pCpfJtfJiWaAp9hFuIiYRkvl2ljb7mRw0D+ivK2tRINxArEery8pGt6X0sVU+H4QV2WBXRNdcQ49nasZNMeNlmiGlCZufvUW3xofMydnlnPFZLNs6SBXMTBoMnjqJv23c029rchkb0o5z2NUxfHOp7XoG5RO+N6fjCKVUHPw1S0= 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=boxqjA03; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=A0JOaiyt; arc=none smtp.client-ip=103.168.172.156 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="boxqjA03"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="A0JOaiyt" Received: from phl-compute-02.internal (phl-compute-02.nyi.internal [10.202.2.42]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 08C2D11519ED; Fri, 23 Aug 2024 10:12:57 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-02.internal (MEProxy); Fri, 23 Aug 2024 10:12:57 -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=fm1; t=1724422377; x=1724508777; bh=ELGASH7j6G b3d9xdzj01RfwRwbVPQlcS6jkM0X3Wqvw=; b=boxqjA03Dj5jk0iqeoJcV1tc8M 4sWfG3fBM0I+/wBUrJRAkAFo2HKJSlWf7ggk8JHodc7Hl/U/28wCPM7znYNWebl8 WYADSqV91tEjZD4Q7dsJygU2LYh3/SNG03nopChYQf+Kt59flSZmKeo1agxtc5H8 HSgTSU+T54ppLIInapCknRWg+Mg39kaOTSMa6/oyWrKp5qwCyOJPZVPnLkpuGdPE +lN3hEBLLPe5g98jpeGkURI99iOKKrCcMAweJnPYyPkNVF+xfaiqDMGo6gB9x5uS Ctl6VtiKFWhbzAr1UJf6LZ1v0h5z8UVUxPRnWyZtX0tscvuOWehsXPWLZTkQ== 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= fm1; t=1724422377; x=1724508777; bh=ELGASH7j6Gb3d9xdzj01RfwRwbVP QlcS6jkM0X3Wqvw=; b=A0JOaiytokpvNdYMnIoBDaYp1acwSBcRfJGGywVGE3Qi 9FWghyfpa9g52NdYsoCmqNvy1P4+8EaOpA7WZt8o5sh18NC6VWFsFRr2bfNfWZh3 +Wj+08cF0ES0Bu2Of4lNuyCfXhjlDWJQ7SHgb+z5GUkj6fa7VxO6dRsJnVWVG4rM zppNajkW4ylva9lbpxaB6UE+7i2BAaM7jkyMYG8XPhT9j8+Ne68Hl5sUeiqYIPAM UJ28gmBJSPcj5F5UPq1ci+Y4l9YIPWWB50302tzBb54SMqtgMvagBMG/1dKj1BmK Kij+MZPeBSeJnELwyiH737Nic8yYgt/3Ie7YX/DJLg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtph htthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhi thesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehpvghffhesphgvfhhfrd hnvght X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:12:55 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ce018738 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:18 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:54 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 09/10] reftable/stack: reorder swapping in the reloaded stack contents Message-ID: <5aee91de25e5bd5198fef307549b58bb4625161f.1724420744.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 code flow of how we swap in the reloaded stack contents is somewhat convoluted because we switch back and forth between swapping in different parts of the stack. Reorder the code to simplify it. We now first close and unlink the old tables which do not get reused before we update the stack to point to the new stack. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 8e85f8b4d99..02472222589 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -273,30 +273,34 @@ static int reftable_stack_reload_once(struct reftable_stack *st, if (err < 0) goto done; - st->readers_len = new_readers_len; - if (st->merged) - reftable_merged_table_free(st->merged); - if (st->readers) { - reftable_free(st->readers); - } - st->readers = new_readers; - new_readers = NULL; - new_readers_len = 0; - - new_merged->suppress_deletions = 1; - st->merged = new_merged; + /* + * Close the old, non-reused readers and proactively try to unlink + * them. This is done for systems like Windows, where the underlying + * file of such an open reader wouldn't have been possible to be + * unlinked by the compacting process. + */ for (i = 0; i < cur_len; i++) { if (cur[i]) { const char *name = reader_name(cur[i]); stack_filename(&table_path, st, name); - reftable_reader_decref(cur[i]); - - /* On Windows, can only unlink after closing. */ unlink(table_path.buf); } } + /* Update the stack to point to the new tables. */ + if (st->merged) + reftable_merged_table_free(st->merged); + new_merged->suppress_deletions = 1; + st->merged = new_merged; + + if (st->readers) + reftable_free(st->readers); + st->readers = new_readers; + st->readers_len = new_readers_len; + new_readers = NULL; + new_readers_len = 0; + done: for (i = 0; i < new_readers_len; i++) reftable_reader_decref(new_readers[i]); From patchwork Fri Aug 23 14:12:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13775288 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 1812818BB9A for ; Fri, 23 Aug 2024 14:13:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422384; cv=none; b=V43Ng9ibPdLi89hFwrfRAb3Wq9XmedVXwpGHGQta+HN9cDNLjkIGfJ3rOJNKZmaRFfwwY2yuHXEI7Je9tFloPkEfyWczleSZrquqWbO9m2jy/lIzqTNh1FdH4EHqyyNT/1GGzqNbhUjjUt4AfScqfwt+cWRYrFAIHw9i/Qx6wL8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724422384; c=relaxed/simple; bh=xvgZQtuMN7A8KhF41cJQlXkBiA2m8w641bGW+GLbWPk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WF0/LZ6WfJ9T2VsuhP8zFxjbP7dxC2q1C9/p8D8MCeYeiF1HANw6kp/izfV5ejwMwfa4ATI7dmaN3cJdI8OEjf4JITSBwC03p9IEPGpLCCMnT71vBxq5JdUiI3rk2s8Fig6+65rAnKfd5EOJAN+FQGvzF0JANdfaseHN4/h8HJA= 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=DUzzS2Le; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=IYk+bQlq; arc=none smtp.client-ip=103.168.172.156 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="DUzzS2Le"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="IYk+bQlq" Received: from phl-compute-06.internal (phl-compute-06.nyi.internal [10.202.2.46]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 668E01151B87; Fri, 23 Aug 2024 10:13:02 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Fri, 23 Aug 2024 10:13:02 -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=fm1; t=1724422382; x=1724508782; bh=0hrv8nM1Id avGFwL6RP9zWOo3xcReikyLTLNX0+pKtk=; b=DUzzS2LeWKX3fSVXyy2bYico2a LvwlZe2+Q3KODiy8smUz8dUY3rTfXk5H9gA2MMQzvU+E9fBCG3AAMLbGSLHmNmz6 md690sTiJa41lp2T5cXVRlFYJLLsqfh0ArwDk2Ys3XzXa/DTZA0KffpaQXg2OIxn 3F711IQ1F0TrgLyLit5tqfRdPYUYfJAegPBJ4uZ8mMZdRGHaOmu7lF6OASbxCZjD 2PoszQPWCJGgmRfsD3PwauMfM6tfK/XQJrcDIqNS6eNnqwnArlZlnN+o3THJU1oq DIo0UT1DaOl3ccqxQzGlMQ61otxsKYnxwM28eOX2WoWpLVIiYcYkhJc5y8Og== 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= fm1; t=1724422382; x=1724508782; bh=0hrv8nM1IdavGFwL6RP9zWOo3xcR eikyLTLNX0+pKtk=; b=IYk+bQlq4TA1XgS7K5AQhCTWAol0oc54iO8MSkrkwUbO jbVDpP10lqqBqSkDV91eSlzSWqv8ALmHfV/NlwqARcGfORcr9k48/be8gH1gf2QD ZsRLcqCCrgqnf9Hbjr/Ddr8DO8MCCoTiR+8bNslik48gckY33o0zH93+Gjbp92fR 9M8FukFhzEsOQmxw0cDP+CsaIx/8j7ZSho9hGwD6MG/eJzHzBKzMWur8bBUezvOM TGWvbO6wXznyjfeJiZrczlHQ+xYfY6/CTnTwvuKxx73Gl/3KeTknGQSCxvZj05Tr 4L3O2BksXe4oGi0xhX8AuHAAJ0/XqlOpPU4ZkoP85w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvvddgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepgedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh dprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtohepphgv fhhfsehpvghffhdrnhgvthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrd horhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Aug 2024 10:13:01 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 5ed3510b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 23 Aug 2024 14:12:24 +0000 (UTC) Date: Fri, 23 Aug 2024 16:12:57 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , karthik nayak , Junio C Hamano Subject: [PATCH v2 10/10] reftable/stack: fix segfault when reload with reused readers fails Message-ID: <4a8d45cc9b49c3351012a36728ee2295b002709a.1724420744.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: It is expected that reloading the stack fails with concurrent writers, e.g. because a table that we just wanted to read just got compacted. In case we decided to reuse readers this will cause a segfault though because we unconditionally release all new readers, including the reused ones. As those are still referenced by the current stack, the result is that we will eventually try to dereference those already-freed readers. Fix this bug by incrementing the refcount of reused readers temporarily. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 23 +++++++++++++++++ reftable/stack_test.c | 59 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/reftable/stack.c b/reftable/stack.c index 02472222589..ce0a35216ba 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -226,6 +226,8 @@ static int reftable_stack_reload_once(struct reftable_stack *st, { size_t cur_len = !st->merged ? 0 : st->merged->readers_len; struct reftable_reader **cur = stack_copy_readers(st, cur_len); + struct reftable_reader **reused = NULL; + size_t reused_len = 0, reused_alloc = 0; size_t names_len = names_length(names); struct reftable_reader **new_readers = reftable_calloc(names_len, sizeof(*new_readers)); @@ -245,6 +247,18 @@ static int reftable_stack_reload_once(struct reftable_stack *st, if (cur[i] && 0 == strcmp(cur[i]->name, name)) { rd = cur[i]; cur[i] = NULL; + + /* + * When reloading the stack fails, we end up + * releasing all new readers. This also + * includes the reused readers, even though + * they are still in used by the old stack. We + * thus need to keep them alive here, which we + * do by bumping their refcount. + */ + REFTABLE_ALLOC_GROW(reused, reused_len + 1, reused_alloc); + reused[reused_len++] = rd; + reftable_reader_incref(rd); break; } } @@ -301,10 +315,19 @@ static int reftable_stack_reload_once(struct reftable_stack *st, new_readers = NULL; new_readers_len = 0; + /* + * Decrement the refcount of reused readers again. This only needs to + * happen on the successful case, because on the unsuccessful one we + * decrement their refcount via `new_readers`. + */ + for (i = 0; i < reused_len; i++) + reftable_reader_decref(reused[i]); + done: for (i = 0; i < new_readers_len; i++) reftable_reader_decref(new_readers[i]); reftable_free(new_readers); + reftable_free(reused); reftable_free(cur); strbuf_release(&table_path); return err; diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 7fb5beb7c94..6809bf9d300 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -10,6 +10,7 @@ license that can be found in the LICENSE file or at #include "system.h" +#include "copy.h" #include "reftable-reader.h" #include "merged.h" #include "basics.h" @@ -1125,6 +1126,63 @@ static void test_reftable_stack_read_across_reload(void) clear_dir(dir); } +static void test_reftable_stack_reload_with_missing_table(void) +{ + struct reftable_write_options opts = { 0 }; + struct reftable_stack *st = NULL; + struct reftable_ref_record rec = { 0 }; + struct reftable_iterator it = { 0 }; + struct strbuf table_path = STRBUF_INIT, content = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + /* Create a first stack and set up an iterator for it. */ + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + write_n_ref_tables(st, 2); + EXPECT(st->merged->readers_len == 2); + reftable_stack_init_ref_iterator(st, &it); + err = reftable_iterator_seek_ref(&it, ""); + EXPECT_ERR(err); + + /* + * Update the tables.list file with some garbage data, while reusing + * our old readers. This should trigger a partial reload of the stack, + * where we try to reuse our old readers. + */ + strbuf_addf(&content, "%s\n", st->readers[0]->name); + strbuf_addf(&content, "%s\n", st->readers[1]->name); + strbuf_addstr(&content, "garbage\n"); + strbuf_addf(&table_path, "%s.lock", st->list_file); + write_file_buf(table_path.buf, content.buf, content.len); + err = rename(table_path.buf, st->list_file); + EXPECT_ERR(err); + + err = reftable_stack_reload(st); + EXPECT(err == -4); + EXPECT(st->merged->readers_len == 2); + + /* + * Even though the reload has failed, we should be able to continue + * using the iterator. + */ + err = reftable_iterator_next_ref(&it, &rec); + EXPECT_ERR(err); + EXPECT(!strcmp(rec.refname, "refs/heads/branch-0000")); + err = reftable_iterator_next_ref(&it, &rec); + EXPECT_ERR(err); + EXPECT(!strcmp(rec.refname, "refs/heads/branch-0001")); + err = reftable_iterator_next_ref(&it, &rec); + EXPECT(err > 0); + + reftable_ref_record_release(&rec); + reftable_iterator_destroy(&it); + reftable_stack_destroy(st); + strbuf_release(&table_path); + strbuf_release(&content); + clear_dir(dir); +} + int stack_test_main(int argc, const char *argv[]) { RUN_TEST(test_empty_add); @@ -1148,6 +1206,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_update_index_check); RUN_TEST(test_reftable_stack_uptodate); RUN_TEST(test_reftable_stack_read_across_reload); + RUN_TEST(test_reftable_stack_reload_with_missing_table); RUN_TEST(test_suggest_compaction_segment); RUN_TEST(test_suggest_compaction_segment_nothing); return 0;