From patchwork Thu Feb 1 07:32: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: 13540713 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 CDA0B1586C8 for ; Thu, 1 Feb 2024 07:32:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772781; cv=none; b=hltlBtb3ChdwQTDv6MDyguifQmlWPVkq5sR26hznBMMMe0kENimOAtVxoA/HQEjJwItg2Zl7yN22d2hNlG9P3YXAEKeA69mUnn1K2QsgahJO1BULpazl8DncTD6xFvpJL3ikxU2redmq8vY/kvp74PbIANo4P3IWUvDWR7UiVe8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772781; c=relaxed/simple; bh=yVkhQYiUZZiYdO6wyCYrIHxMbk/V5KPalqXmk7QkPCA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DyrKBADyNitTsM+U6dKc/2QYcFrcnxOWHl0Ny2Daj1p8+BkJpyJhPG+VOnt/mBW5seV4hc1LMi4Ku+RNTdtePrjmK2EAK3A3I1hdEv2+YS1Cj6vBLCEoggdDuTF+VWi19TYczC2fZY8DdakgiuKDreLfl6gl5riBGDhf4w0/Gww= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=X2qHTuXn; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=RW44S5eF; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="X2qHTuXn"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="RW44S5eF" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.west.internal (Postfix) with ESMTP id BA6E31800085; Thu, 1 Feb 2024 02:32:58 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 01 Feb 2024 02:32:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772778; x=1706859178; bh=NN0H941iUo c5LY/AP6Rkq4xf1NbqBDFq+6Q03yrzUlk=; b=X2qHTuXnM2uwTqydJRHEREy2Om CjnzHo4bCTIPBkkUcFP0rSCNRJCPwya8PeEfftBNEv7HGcXh+Ria45s94wuT+p+s jJ/khwZsl/4oQtFVR9LDqo4IAkXmIX6hvt+JtyWdRuXCkvJ/QRPlq8piYFMC4x4v nbXVmqMB5+EDvQ4CIVa2bYosGVueO/lVnFkcXbqaPI51o71pJAw1Om6qL0Hni6EW C4UEHH8qrZ4lOoY5kNcRGvvyIJ7xMV5fUklBOEnw68NKH23HFgQ2kZY1Q19qnQQC MdNaWBFflPpms0GBnAUML2d3WvhOUseOxAOWyXwxc51W59GOi298m3gOsdHw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772778; x=1706859178; bh=NN0H941iUoc5LY/AP6Rkq4xf1Nbq BDFq+6Q03yrzUlk=; b=RW44S5eFbvyxBTRKlEQAj3SlGj6ForetUgFv+MqgP3gU A7Nv1yW9ID0o2kMC2A2SHrvCElYs4U24iEXy0Gx1W2HvzHMrIVYGGT8/vxBs8pj4 TPGnJXiFLGNNTXPuRL2MZZhZUo2+1Uqeag6ND9Ip86rYJzJAHUz1O4AgSfLg8UZI 5chdK2mEVjuyns1aEr6ttSDKb+E2t15wfpa0SHzCJvN/BAfW3TN2X6E790IqywCB +HLx1P7iOXpnVJSWwIFuMo1Ojk7Vbsf3C6mB1A2BqVN7sNMSCLlCQ/AskmXFK73Z 22xMM+P1K4rMt0AcK4chgmSkYfU/4ea/iHG8vq29UQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:32:57 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 05ccdddf (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:34 +0000 (UTC) Date: Thu, 1 Feb 2024 08:32:54 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 1/9] reftable: introduce macros to grow arrays Message-ID: <12bd721ddff7020eb9e9ebd4e797d50193250bc0.1706772591.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: Throughout the reftable library we have many cases where we need to grow arrays. In order to avoid too many reallocations, we roughly double the capacity of the array on each iteration. The resulting code pattern is duplicated across many sites. We have similar patterns in our main codebase, which is why we have eventually introduced an `ALLOC_GROW()` macro to abstract it away and avoid some code duplication. We cannot easily reuse this macro here though because `ALLOC_GROW()` uses `REALLOC_ARRAY()`, which in turn will call realloc(3P) to grow the array. The reftable code is structured as a library though (even if the boundaries are fuzzy), and one property this brings with it is that it is possible to plug in your own allocators. So instead of using realloc(3P), we need to use `reftable_realloc()` that knows to use the user-provided implementation. So let's introduce two new macros `REFTABLE_REALLOC_ARRAY()` and `REFTABLE_ALLOC_GROW()` that mirror what we do in our main codebase, with two modifications: - They use `reftable_realloc()`, as explained above. - They use a different growth factor of `2 * cap + 1` instead of `(cap + 16) * 3 / 2`. The second change is because we know a bit more about the allocation patterns in the reftable library. In most cases, we end up only having a handful of items in the array and don't end up growing them. The initial capacity that our normal growth factor uses (which is 24) would thus end up over-allocating in a lot of code paths. This effect is measurable: - Before change: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,843,446 allocs, 3,843,294 frees, 223,761,402 bytes allocated - After change with a growth factor of `(2 * alloc + 1)`: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,843,446 allocs, 3,843,294 frees, 223,761,410 bytes allocated - After change with a growth factor of `(alloc + 16)* 2 / 3`: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,833,673 allocs, 3,833,521 frees, 4,728,251,742 bytes allocated While the total heap usage is roughly the same, we do end up allocating significantly more bytes with our usual growth factor (in fact, roughly 21 times as many). Convert the reftable library to use these new macros. Signed-off-by: Patrick Steinhardt --- reftable/basics.c | 8 ++------ reftable/basics.h | 11 +++++++++++ reftable/block.c | 7 +------ reftable/merged_test.c | 20 ++++++-------------- reftable/pq.c | 8 ++------ reftable/stack.c | 29 ++++++++++++----------------- reftable/writer.c | 14 ++------------ 7 files changed, 36 insertions(+), 61 deletions(-) diff --git a/reftable/basics.c b/reftable/basics.c index f761e48028..af9004cec2 100644 --- a/reftable/basics.c +++ b/reftable/basics.c @@ -89,17 +89,13 @@ void parse_names(char *buf, int size, char ***namesp) next = end; } if (p < next) { - if (names_len == names_cap) { - names_cap = 2 * names_cap + 1; - names = reftable_realloc( - names, names_cap * sizeof(*names)); - } + REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap); names[names_len++] = xstrdup(p); } p = next + 1; } - names = reftable_realloc(names, (names_len + 1) * sizeof(*names)); + REFTABLE_REALLOC_ARRAY(names, names_len + 1); names[names_len] = NULL; *namesp = names; } diff --git a/reftable/basics.h b/reftable/basics.h index 096b36862b..2f855cd724 100644 --- a/reftable/basics.h +++ b/reftable/basics.h @@ -53,6 +53,17 @@ void *reftable_realloc(void *p, size_t sz); void reftable_free(void *p); void *reftable_calloc(size_t sz); +#define REFTABLE_REALLOC_ARRAY(x, alloc) (x) = reftable_realloc((x), st_mult(sizeof(*(x)), (alloc))) +#define REFTABLE_ALLOC_GROW(x, nr, alloc) \ + do { \ + if ((nr) > alloc) { \ + alloc = 2 * (alloc) + 1; \ + if (alloc < (nr)) \ + alloc = (nr); \ + REFTABLE_REALLOC_ARRAY(x, alloc); \ + } \ + } while (0) + /* Find the longest shared prefix size of `a` and `b` */ struct strbuf; int common_prefix_size(struct strbuf *a, struct strbuf *b); diff --git a/reftable/block.c b/reftable/block.c index 1df3d8a0f0..6952d0facf 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -51,12 +51,7 @@ static int block_writer_register_restart(struct block_writer *w, int n, if (2 + 3 * rlen + n > w->block_size - w->next) return -1; if (is_restart) { - if (w->restart_len == w->restart_cap) { - w->restart_cap = w->restart_cap * 2 + 1; - w->restarts = reftable_realloc( - w->restarts, sizeof(uint32_t) * w->restart_cap); - } - + REFTABLE_ALLOC_GROW(w->restarts, w->restart_len + 1, w->restart_cap); w->restarts[w->restart_len++] = w->next; } diff --git a/reftable/merged_test.c b/reftable/merged_test.c index 46908f738f..e05351e035 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -231,14 +231,10 @@ static void test_merged(void) while (len < 100) { /* cap loops/recursion. */ struct reftable_ref_record ref = { NULL }; int err = reftable_iterator_next_ref(&it, &ref); - if (err > 0) { + if (err > 0) break; - } - if (len == cap) { - cap = 2 * cap + 1; - out = reftable_realloc( - out, sizeof(struct reftable_ref_record) * cap); - } + + REFTABLE_ALLOC_GROW(out, len + 1, cap); out[len++] = ref; } reftable_iterator_destroy(&it); @@ -368,14 +364,10 @@ static void test_merged_logs(void) while (len < 100) { /* cap loops/recursion. */ struct reftable_log_record log = { NULL }; int err = reftable_iterator_next_log(&it, &log); - if (err > 0) { + if (err > 0) break; - } - if (len == cap) { - cap = 2 * cap + 1; - out = reftable_realloc( - out, sizeof(struct reftable_log_record) * cap); - } + + REFTABLE_ALLOC_GROW(out, len + 1, cap); out[len++] = log; } reftable_iterator_destroy(&it); diff --git a/reftable/pq.c b/reftable/pq.c index dcefeb793a..2461daf5ff 100644 --- a/reftable/pq.c +++ b/reftable/pq.c @@ -75,13 +75,9 @@ void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry { int i = 0; - if (pq->len == pq->cap) { - pq->cap = 2 * pq->cap + 1; - pq->heap = reftable_realloc(pq->heap, - pq->cap * sizeof(struct pq_entry)); - } - + REFTABLE_ALLOC_GROW(pq->heap, pq->len + 1, pq->cap); pq->heap[pq->len++] = *e; + i = pq->len - 1; while (i > 0) { int j = (i - 1) / 2; diff --git a/reftable/stack.c b/reftable/stack.c index bf3869ce70..1dfab99e96 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -551,7 +551,7 @@ struct reftable_addition { struct reftable_stack *stack; char **new_tables; - int new_tables_len; + size_t new_tables_len, new_tables_cap; uint64_t next_update_index; }; @@ -602,8 +602,9 @@ static int reftable_stack_init_addition(struct reftable_addition *add, static void reftable_addition_close(struct reftable_addition *add) { - int i = 0; struct strbuf nm = STRBUF_INIT; + size_t i; + for (i = 0; i < add->new_tables_len; i++) { stack_filename(&nm, add->stack, add->new_tables[i]); unlink(nm.buf); @@ -613,6 +614,7 @@ static void reftable_addition_close(struct reftable_addition *add) reftable_free(add->new_tables); add->new_tables = NULL; add->new_tables_len = 0; + add->new_tables_cap = 0; delete_tempfile(&add->lock_file); strbuf_release(&nm); @@ -631,8 +633,8 @@ int reftable_addition_commit(struct reftable_addition *add) { struct strbuf table_list = STRBUF_INIT; int lock_file_fd = get_tempfile_fd(add->lock_file); - int i = 0; int err = 0; + size_t i; if (add->new_tables_len == 0) goto done; @@ -660,12 +662,12 @@ int reftable_addition_commit(struct reftable_addition *add) } /* success, no more state to clean up. */ - for (i = 0; i < add->new_tables_len; i++) { + for (i = 0; i < add->new_tables_len; i++) reftable_free(add->new_tables[i]); - } reftable_free(add->new_tables); add->new_tables = NULL; add->new_tables_len = 0; + add->new_tables_cap = 0; err = reftable_stack_reload_maybe_reuse(add->stack, 1); if (err) @@ -792,11 +794,9 @@ int reftable_addition_add(struct reftable_addition *add, goto done; } - add->new_tables = reftable_realloc(add->new_tables, - sizeof(*add->new_tables) * - (add->new_tables_len + 1)); - add->new_tables[add->new_tables_len] = strbuf_detach(&next_name, NULL); - add->new_tables_len++; + REFTABLE_ALLOC_GROW(add->new_tables, add->new_tables_len + 1, + add->new_tables_cap); + add->new_tables[add->new_tables_len++] = strbuf_detach(&next_name, NULL); done: if (tab_fd > 0) { close(tab_fd); @@ -1367,17 +1367,12 @@ static int stack_check_addition(struct reftable_stack *st, while (1) { struct reftable_ref_record ref = { NULL }; err = reftable_iterator_next_ref(&it, &ref); - if (err > 0) { + if (err > 0) break; - } if (err < 0) goto done; - if (len >= cap) { - cap = 2 * cap + 1; - refs = reftable_realloc(refs, cap * sizeof(refs[0])); - } - + REFTABLE_ALLOC_GROW(refs, len + 1, cap); refs[len++] = ref; } diff --git a/reftable/writer.c b/reftable/writer.c index ee4590e20f..4483bb21c3 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -200,12 +200,7 @@ static void writer_index_hash(struct reftable_writer *w, struct strbuf *hash) return; } - if (key->offset_len == key->offset_cap) { - key->offset_cap = 2 * key->offset_cap + 1; - key->offsets = reftable_realloc( - key->offsets, sizeof(uint64_t) * key->offset_cap); - } - + REFTABLE_ALLOC_GROW(key->offsets, key->offset_len + 1, key->offset_cap); key->offsets[key->offset_len++] = off; } @@ -674,12 +669,7 @@ static int writer_flush_nonempty_block(struct reftable_writer *w) if (err < 0) return err; - if (w->index_cap == w->index_len) { - w->index_cap = 2 * w->index_cap + 1; - w->index = reftable_realloc( - w->index, - sizeof(struct reftable_index_record) * w->index_cap); - } + REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap); ir.offset = w->next; strbuf_reset(&ir.last_key); From patchwork Thu Feb 1 07:32:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540714 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 02CFE158D9D for ; Thu, 1 Feb 2024 07:33:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772785; cv=none; b=mXfpNegFLFgDwK/SAgdX5uQlRR6enG6GRk/+5jy6WGqbTQPgj2IvBz+1mnxYT/myuxqIQl1YO2PKZPWwGz1Y83nFCoPtZuCfkIZhuRj3dcdVHFrozcGX/t5kzCNGjRmWEmFRgv3wP6KQp0u0UDKyDbZEg+dqhCa5cCpPIDsoB/4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772785; c=relaxed/simple; bh=CI74UhAWW//ibkliJwkRJbxN66qAAGoeim2w9J9h0BY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=quGFn6Xa4zb1Xk8LBS69SRdKiZ8DGXtvE0PIySuvj50HoPeCZCx+6HiWKlozW+W4gR7gOTT2LTTn2A+1hM78Eu/AHIG2EZLYiS+UkY3hEDZZIFmh5AvmGwIeQuCCJOHWS3xR4gBDAGN4y4UaZtR9FQxyUdoVSUXWqArjIjMc2lI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=ibgGMVMa; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=eSZqUEIn; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="ibgGMVMa"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="eSZqUEIn" Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfhigh.west.internal (Postfix) with ESMTP id DEC521800085; Thu, 1 Feb 2024 02:33:01 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 01 Feb 2024 02:33:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772781; x=1706859181; bh=/baS00s2x0 xo6vipX22IiF8Xbf1nQscP95sr9tl1rdI=; b=ibgGMVMaDkxHgns/GmrrnqqnJh f0CrJVfoNTqBntnjH60psTpvjY4bGxYumVTgLVmVMpjlCSkL+7GTGePSDMQzS5FF dPca2m1hYppUemBIQh7r0osQP2pmlogEz1lS/yEtzT56kstEEm7AUTgoGh9yne57 OMHRmmulxihyq/eTqdGEHesRfgYnWQ1XaJEKF/bAYrJPvxv6TBekrLgnH3AixYpH fKh5jKwtOeP86irYE9HUgmymKrIzMofssVfZWAoPAVe9hSDR9Dm/uuwoaKoZJvLm NqqfV21V1yYAgXzmavVrf9bvupiFpTviqR+cmmftYD97YiMe0tDtVVaprprQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772781; x=1706859181; bh=/baS00s2x0xo6vipX22IiF8Xbf1n QscP95sr9tl1rdI=; b=eSZqUEInRdDGJhHRGwhTN1KFlMRVgbRm58eg2RG7LQO7 E5SUgdJv5Bu4l7xdVzDIDrS0OwZOHxA697b1g1m4lEq4xj+T+upm+DnNZ83pEI5g 2xDzkTilNYMABeh1It9tLzdRL29Bgx5kprVv8mbZlG0+o3/7wkn7B1zethmJXxEq wmrMf16BzgiYT7W4ZKO1j0JTWegHq5lc91oCRJrYMgWYeLZqn2fHR+4fl1ymKDhi iUnbXh1A0MyT6GZzKVsNLK2RFzC2Zf/MlHHauiFJTulczMYDKvSmHqXU78NZumLs w79r/wVDGKJdn5+vLDeJbH958PRQxIt3sRFwwqtKbA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:00 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 8cc733b3 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:38 +0000 (UTC) Date: Thu, 1 Feb 2024 08:32:58 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 2/9] reftable: introduce macros to allocate arrays Message-ID: <2dde581a0256e4634ca4a64f313a98204763906a.1706772591.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: Similar to the preceding commit, let's carry over macros to allocate arrays with `REFTABLE_ALLOC_ARRAY()` and `REFTABLE_CALLOC_ARRAY()`. This requires us to change the signature of `reftable_calloc()`, which only takes a single argument right now and thus puts the burden on the caller to calculate the final array's size. This is a net improvement though as it means that we can now provide proper overflow checks when multiplying the array size with the member size. Convert callsites of `reftable_calloc()` to the new signature, using the new macros where possible. Signed-off-by: Patrick Steinhardt --- reftable/basics.h | 4 +++- reftable/block_test.c | 2 +- reftable/blocksource.c | 4 ++-- reftable/iter.c | 3 +-- reftable/merged.c | 4 ++-- reftable/merged_test.c | 22 +++++++++++++--------- reftable/publicbasics.c | 3 ++- reftable/reader.c | 8 +++----- reftable/readwrite_test.c | 8 +++++--- reftable/record_test.c | 4 ++-- reftable/refname.c | 4 ++-- reftable/stack.c | 26 ++++++++++++-------------- reftable/tree.c | 4 ++-- reftable/writer.c | 7 +++---- 14 files changed, 53 insertions(+), 50 deletions(-) diff --git a/reftable/basics.h b/reftable/basics.h index 2f855cd724..4c3ac963a3 100644 --- a/reftable/basics.h +++ b/reftable/basics.h @@ -51,8 +51,10 @@ int names_length(char **names); void *reftable_malloc(size_t sz); void *reftable_realloc(void *p, size_t sz); void reftable_free(void *p); -void *reftable_calloc(size_t sz); +void *reftable_calloc(size_t nelem, size_t elsize); +#define REFTABLE_ALLOC_ARRAY(x, alloc) (x) = reftable_malloc(st_mult(sizeof(*(x)), (alloc))) +#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x))) #define REFTABLE_REALLOC_ARRAY(x, alloc) (x) = reftable_realloc((x), st_mult(sizeof(*(x)), (alloc))) #define REFTABLE_ALLOC_GROW(x, nr, alloc) \ do { \ diff --git a/reftable/block_test.c b/reftable/block_test.c index dedb05c7d8..e162c6e33f 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -36,7 +36,7 @@ static void test_block_read_write(void) int j = 0; struct strbuf want = STRBUF_INIT; - block.data = reftable_calloc(block_size); + REFTABLE_CALLOC_ARRAY(block.data, block_size); block.len = block_size; block.source = malloc_block_source(); block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size, diff --git a/reftable/blocksource.c b/reftable/blocksource.c index 8c41e3c70f..eeed254ba9 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -29,7 +29,7 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off, { struct strbuf *b = v; assert(off + size <= b->len); - dest->data = reftable_calloc(size); + REFTABLE_CALLOC_ARRAY(dest->data, size); memcpy(dest->data, b->buf + off, size); dest->len = size; return size; @@ -132,7 +132,7 @@ int reftable_block_source_from_file(struct reftable_block_source *bs, return REFTABLE_IO_ERROR; } - p = reftable_calloc(sizeof(*p)); + REFTABLE_CALLOC_ARRAY(p, 1); p->size = st.st_size; p->data = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); close(fd); diff --git a/reftable/iter.c b/reftable/iter.c index a8d174c040..8b5ebf6183 100644 --- a/reftable/iter.c +++ b/reftable/iter.c @@ -160,8 +160,7 @@ int new_indexed_table_ref_iter(struct indexed_table_ref_iter **dest, int oid_len, uint64_t *offsets, int offset_len) { struct indexed_table_ref_iter empty = INDEXED_TABLE_REF_ITER_INIT; - struct indexed_table_ref_iter *itr = - reftable_calloc(sizeof(struct indexed_table_ref_iter)); + struct indexed_table_ref_iter *itr = reftable_calloc(1, sizeof(*itr)); int err = 0; *itr = empty; diff --git a/reftable/merged.c b/reftable/merged.c index c258ce953e..2031fd51b4 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -190,7 +190,7 @@ int reftable_new_merged_table(struct reftable_merged_table **dest, } } - m = reftable_calloc(sizeof(struct reftable_merged_table)); + REFTABLE_CALLOC_ARRAY(m, 1); m->stack = stack; m->stack_len = n; m->min = first_min; @@ -240,7 +240,7 @@ static int merged_table_seek_record(struct reftable_merged_table *mt, struct reftable_record *rec) { struct reftable_iterator *iters = reftable_calloc( - sizeof(struct reftable_iterator) * mt->stack_len); + mt->stack_len, sizeof(*iters)); struct merged_iter merged = { .stack = iters, .typ = reftable_record_type(rec), diff --git a/reftable/merged_test.c b/reftable/merged_test.c index e05351e035..e233a9d581 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -93,10 +93,12 @@ merged_table_from_records(struct reftable_ref_record **refs, int i = 0; struct reftable_merged_table *mt = NULL; int err; - struct reftable_table *tabs = - reftable_calloc(n * sizeof(struct reftable_table)); - *readers = reftable_calloc(n * sizeof(struct reftable_reader *)); - *source = reftable_calloc(n * sizeof(**source)); + struct reftable_table *tabs; + + REFTABLE_CALLOC_ARRAY(tabs, n); + REFTABLE_CALLOC_ARRAY(*readers, n); + REFTABLE_CALLOC_ARRAY(*source, n); + for (i = 0; i < n; i++) { write_test_table(&buf[i], refs[i], sizes[i]); block_source_from_strbuf(&(*source)[i], &buf[i]); @@ -266,10 +268,12 @@ merged_table_from_log_records(struct reftable_log_record **logs, int i = 0; struct reftable_merged_table *mt = NULL; int err; - struct reftable_table *tabs = - reftable_calloc(n * sizeof(struct reftable_table)); - *readers = reftable_calloc(n * sizeof(struct reftable_reader *)); - *source = reftable_calloc(n * sizeof(**source)); + struct reftable_table *tabs; + + REFTABLE_CALLOC_ARRAY(tabs, n); + REFTABLE_CALLOC_ARRAY(*readers, n); + REFTABLE_CALLOC_ARRAY(*source, n); + for (i = 0; i < n; i++) { write_test_log_table(&buf[i], logs[i], sizes[i], i + 1); block_source_from_strbuf(&(*source)[i], &buf[i]); @@ -412,7 +416,7 @@ static void test_default_write_opts(void) }; int err; struct reftable_block_source source = { NULL }; - struct reftable_table *tab = reftable_calloc(sizeof(*tab) * 1); + struct reftable_table *tab = reftable_calloc(1, sizeof(*tab)); uint32_t hash_id; struct reftable_reader *rd = NULL; struct reftable_merged_table *merged = NULL; diff --git a/reftable/publicbasics.c b/reftable/publicbasics.c index bcb82530d6..44b84a125e 100644 --- a/reftable/publicbasics.c +++ b/reftable/publicbasics.c @@ -37,8 +37,9 @@ void reftable_free(void *p) free(p); } -void *reftable_calloc(size_t sz) +void *reftable_calloc(size_t nelem, size_t elsize) { + size_t sz = st_mult(nelem, elsize); void *p = reftable_malloc(sz); memset(p, 0, sz); return p; diff --git a/reftable/reader.c b/reftable/reader.c index 64dc366fb1..3e0de5e8ad 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -539,8 +539,7 @@ static int reader_seek_indexed(struct reftable_reader *r, if (err == 0) { struct table_iter empty = TABLE_ITER_INIT; - struct table_iter *malloced = - reftable_calloc(sizeof(struct table_iter)); + struct table_iter *malloced = reftable_calloc(1, sizeof(*malloced)); *malloced = empty; table_iter_copy_from(malloced, &next); iterator_from_table_iter(it, malloced); @@ -635,8 +634,7 @@ void reader_close(struct reftable_reader *r) int reftable_new_reader(struct reftable_reader **p, struct reftable_block_source *src, char const *name) { - struct reftable_reader *rd = - reftable_calloc(sizeof(struct reftable_reader)); + struct reftable_reader *rd = reftable_calloc(1, sizeof(*rd)); int err = init_reader(rd, src, name); if (err == 0) { *p = rd; @@ -711,7 +709,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, uint8_t *oid) { struct table_iter ti_empty = TABLE_ITER_INIT; - struct table_iter *ti = reftable_calloc(sizeof(struct table_iter)); + struct table_iter *ti = reftable_calloc(1, sizeof(*ti)); struct filtering_ref_iterator *filter = NULL; struct filtering_ref_iterator empty = FILTERING_REF_ITERATOR_INIT; int oid_len = hash_size(r->hash_id); diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index b8a3224016..91ea7a10ef 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -56,7 +56,9 @@ static void write_table(char ***names, struct strbuf *buf, int N, int i = 0, n; struct reftable_log_record log = { NULL }; const struct reftable_stats *stats = NULL; - *names = reftable_calloc(sizeof(char *) * (N + 1)); + + REFTABLE_CALLOC_ARRAY(*names, N + 1); + reftable_writer_set_limits(w, update_index, update_index); for (i = 0; i < N; i++) { char name[100]; @@ -188,7 +190,7 @@ static void test_log_overflow(void) static void test_log_write_read(void) { int N = 2; - char **names = reftable_calloc(sizeof(char *) * (N + 1)); + char **names = reftable_calloc(N + 1, sizeof(*names)); int err; struct reftable_write_options opts = { .block_size = 256, @@ -519,7 +521,7 @@ static void test_table_read_write_seek_index(void) static void test_table_refs_for(int indexed) { int N = 50; - char **want_names = reftable_calloc(sizeof(char *) * (N + 1)); + char **want_names = reftable_calloc(N + 1, sizeof(*want_names)); int want_names_len = 0; uint8_t want_hash[GIT_SHA1_RAWSZ]; diff --git a/reftable/record_test.c b/reftable/record_test.c index 2876db7d27..999366ad46 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -231,8 +231,8 @@ static void test_reftable_log_record_roundtrip(void) .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { - .new_hash = reftable_calloc(GIT_SHA1_RAWSZ), - .old_hash = reftable_calloc(GIT_SHA1_RAWSZ), + .new_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1), + .old_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1), .name = xstrdup("old name"), .email = xstrdup("old@email"), .message = xstrdup("old message"), diff --git a/reftable/refname.c b/reftable/refname.c index 9573496932..7570e4acf9 100644 --- a/reftable/refname.c +++ b/reftable/refname.c @@ -140,8 +140,8 @@ int validate_ref_record_addition(struct reftable_table tab, { struct modification mod = { .tab = tab, - .add = reftable_calloc(sizeof(char *) * sz), - .del = reftable_calloc(sizeof(char *) * sz), + .add = reftable_calloc(sz, sizeof(*mod.add)), + .del = reftable_calloc(sz, sizeof(*mod.del)), }; int i = 0; int err = 0; diff --git a/reftable/stack.c b/reftable/stack.c index 1dfab99e96..d084823a92 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -50,8 +50,7 @@ static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) int reftable_new_stack(struct reftable_stack **dest, const char *dir, struct reftable_write_options config) { - struct reftable_stack *p = - reftable_calloc(sizeof(struct reftable_stack)); + struct reftable_stack *p = reftable_calloc(1, sizeof(*p)); struct strbuf list_file_name = STRBUF_INIT; int err = 0; @@ -114,7 +113,7 @@ int read_lines(const char *filename, char ***namesp) int err = 0; if (fd < 0) { if (errno == ENOENT) { - *namesp = reftable_calloc(sizeof(char *)); + REFTABLE_CALLOC_ARRAY(*namesp, 1); return 0; } @@ -191,8 +190,7 @@ void reftable_stack_destroy(struct reftable_stack *st) static struct reftable_reader **stack_copy_readers(struct reftable_stack *st, int cur_len) { - struct reftable_reader **cur = - reftable_calloc(sizeof(struct reftable_reader *) * cur_len); + struct reftable_reader **cur = reftable_calloc(cur_len, sizeof(*cur)); int i = 0; for (i = 0; i < cur_len; i++) { cur[i] = st->readers[i]; @@ -208,9 +206,9 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, int err = 0; int names_len = names_length(names); struct reftable_reader **new_readers = - reftable_calloc(sizeof(struct reftable_reader *) * names_len); + reftable_calloc(names_len, sizeof(*new_readers)); struct reftable_table *new_tables = - reftable_calloc(sizeof(struct reftable_table) * names_len); + reftable_calloc(names_len, sizeof(*new_tables)); int new_readers_len = 0; struct reftable_merged_table *new_merged = NULL; struct strbuf table_path = STRBUF_INIT; @@ -344,7 +342,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st, goto out; } - names = reftable_calloc(sizeof(char *)); + REFTABLE_CALLOC_ARRAY(names, 1); } else { err = fd_read_lines(fd, &names); if (err < 0) @@ -686,7 +684,7 @@ int reftable_stack_new_addition(struct reftable_addition **dest, { int err = 0; struct reftable_addition empty = REFTABLE_ADDITION_INIT; - *dest = reftable_calloc(sizeof(**dest)); + REFTABLE_CALLOC_ARRAY(*dest, 1); **dest = empty; err = reftable_stack_init_addition(*dest, st); if (err) { @@ -871,7 +869,7 @@ static int stack_write_compact(struct reftable_stack *st, { int subtabs_len = last - first + 1; struct reftable_table *subtabs = reftable_calloc( - sizeof(struct reftable_table) * (last - first + 1)); + last - first + 1, sizeof(*subtabs)); struct reftable_merged_table *mt = NULL; int err = 0; struct reftable_iterator it = { NULL }; @@ -979,9 +977,9 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, int compact_count = last - first + 1; char **listp = NULL; char **delete_on_success = - reftable_calloc(sizeof(char *) * (compact_count + 1)); + reftable_calloc(compact_count + 1, sizeof(*delete_on_success)); char **subtable_locks = - reftable_calloc(sizeof(char *) * (compact_count + 1)); + reftable_calloc(compact_count + 1, sizeof(*subtable_locks)); int i = 0; int j = 0; int is_empty_table = 0; @@ -1204,7 +1202,7 @@ int fastlog2(uint64_t sz) struct segment *sizes_to_segments(int *seglen, uint64_t *sizes, int n) { - struct segment *segs = reftable_calloc(sizeof(struct segment) * n); + struct segment *segs = reftable_calloc(n, sizeof(*segs)); int next = 0; struct segment cur = { 0 }; int i = 0; @@ -1268,7 +1266,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, int n) static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st) { uint64_t *sizes = - reftable_calloc(sizeof(uint64_t) * st->merged->stack_len); + reftable_calloc(st->merged->stack_len, sizeof(*sizes)); int version = (st->config.hash_id == GIT_SHA1_FORMAT_ID) ? 1 : 2; int overhead = header_size(version) - 1; int i = 0; diff --git a/reftable/tree.c b/reftable/tree.c index a5bf880985..528f33ae38 100644 --- a/reftable/tree.c +++ b/reftable/tree.c @@ -20,8 +20,8 @@ struct tree_node *tree_search(void *key, struct tree_node **rootp, if (!insert) { return NULL; } else { - struct tree_node *n = - reftable_calloc(sizeof(struct tree_node)); + struct tree_node *n; + REFTABLE_CALLOC_ARRAY(n, 1); n->key = key; *rootp = n; return *rootp; diff --git a/reftable/writer.c b/reftable/writer.c index 4483bb21c3..47b3448132 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -49,7 +49,7 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len, { int n = 0; if (w->pending_padding > 0) { - uint8_t *zeroed = reftable_calloc(w->pending_padding); + uint8_t *zeroed = reftable_calloc(w->pending_padding, sizeof(*zeroed)); int n = w->write(w->write_arg, zeroed, w->pending_padding); if (n < 0) return n; @@ -123,8 +123,7 @@ struct reftable_writer * reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t), void *writer_arg, struct reftable_write_options *opts) { - struct reftable_writer *wp = - reftable_calloc(sizeof(struct reftable_writer)); + struct reftable_writer *wp = reftable_calloc(1, sizeof(*wp)); strbuf_init(&wp->block_writer_data.last_key, 0); options_set_defaults(opts); if (opts->block_size >= (1 << 24)) { @@ -132,7 +131,7 @@ reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t), abort(); } wp->last_key = reftable_empty_strbuf; - wp->block = reftable_calloc(opts->block_size); + REFTABLE_CALLOC_ARRAY(wp->block, opts->block_size); wp->write = writer_func; wp->write_arg = writer_arg; wp->opts = *opts; From patchwork Thu Feb 1 07:33:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540715 Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) (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 1586C158D9D for ; Thu, 1 Feb 2024 07:33:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.24 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772788; cv=none; b=bBSGkqY5QFq8+IEMorkTVuPVXcNDKyZL0CjCEA/V0JzKlT64O3GMuwrfJiYwoyZGSKeRKjOOOzlTSrmdxeK/2BpAPsV+Wlalm5tgQc+Uy3ph9F+oxc/bwHpupGRc2a1lrH5zqjiOvpT35RCAUpzIn2ITsfSG2opaFbwocGAv4Oo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772788; c=relaxed/simple; bh=3eBTWL+UOwZNVKCwAJiHZydd9vY+oVSJ1xcN0z/bTdo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oGs6fEUhwAx4ZmEAtm4aiPQxXNFOJ5oRyxP6fj8mjk9TTgFzb/+l7LM56RsUO1fpPsSASUFDUQmuf7hjU3ohVYtM7Oeu0HmpOjo0oUO7v3L/JMHo9gs1Qj6agtUsVGjrgkX9oNrcz+1MEzVzLQ7KaXIzy8M/lWCKBpggdnU1DSU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=HM8Yth8a; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=DiAOVJt4; arc=none smtp.client-ip=64.147.123.24 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="HM8Yth8a"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="DiAOVJt4" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id 22C653200AF0; Thu, 1 Feb 2024 02:33:06 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 01 Feb 2024 02:33:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772785; x=1706859185; bh=txa/XPJlhl FGVcKVuBWCooph8usBSwB9ODxCKkwx9fs=; b=HM8Yth8aA9uMio1CjKb4hDKK6V 1LCO5Z7vsw4PTpaKg4KI0bsvfA5XAA1nWP7nuPfr5j/Rhim5jP+K2mJsbnzouXee Ptd5mZgICqf8n51rqIyuEPn18gw3BmO7cMgF74Sbv4cmQzxX2OX5KkEPsIKEI4GV AmxA5GCcF1B8nQRg2986zxDeAsJOoeBtl0R4dRTmks60uQfOXSqT5xKC/YJpHhj7 /S8M0U1PhtSmS8X8u3EzGjf7u8d3SprxuEmHoHMLf6E5yg6lGdkEI5zsSnZrw7lu ndDPZKBllgfQuGHClfJcFDPU9L9fWdl2a1fU1C32jQt6GB8fB9byOieZxDNg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772785; x=1706859185; bh=txa/XPJlhlFGVcKVuBWCooph8usB SwB9ODxCKkwx9fs=; b=DiAOVJt4B+AKf4eMlQ2S5mh2eTawD4JIoekqauwDzg1U oGoLwGPeabOlDB/g5jOn8LkX+TM+sYF8KQo0Mbd71IhEj/k8uS57RgqxznHBAzs2 xvnWbT+VUwYHzhHREyMhtTSen3taAB6IEmky/eVYWYkw64JMt227itxgmAB9ExP8 ZSIGHuxpIJbtt9JnCDZ8Qj/BoXGlrqFwqm+CwlP6x+Xoye2hIQfC3EQJyoUwTxvv ZJWMJFXAmsoUBB4UWSknHXXE+j+b9PoCzjUDLXaMpk4m+3s1vUm1O7QSNYf2pMxV ZWESq1PoNrRjbMD846HO0GBf99DrL6AcRdyaMXfJcQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:04 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ab6d49e6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:42 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:02 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 3/9] reftable/stack: fix parameter validation when compacting range 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 `stack_compact_range()` function receives a "first" and "last" index that indicates which tables of the reftable stack should be compacted. Naturally, "first" must be smaller than "last" in order to identify a proper range of tables to compress, which we indeed also assert in the function. But the validations happens after we have already allocated arrays with a size of `last - first + 1`, leading to an underflow and thus an invalid allocation size. Fix this by reordering the array allocations to happen after we have validated parameters. While at it, convert the array allocations to use the newly introduced macros. Note that the relevant variables pointing into arrays should also be converted to use `size_t` instead of `int`. This is left for a later commit in this series. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index d084823a92..b6b24c90bf 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -966,6 +966,7 @@ static int stack_write_compact(struct reftable_stack *st, static int stack_compact_range(struct reftable_stack *st, int first, int last, struct reftable_log_expiry_config *expiry) { + char **delete_on_success = NULL, **subtable_locks = NULL, **listp = NULL; struct strbuf temp_tab_file_name = STRBUF_INIT; struct strbuf new_table_name = STRBUF_INIT; struct strbuf lock_file_name = STRBUF_INIT; @@ -974,12 +975,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, int err = 0; int have_lock = 0; int lock_file_fd = -1; - int compact_count = last - first + 1; - char **listp = NULL; - char **delete_on_success = - reftable_calloc(compact_count + 1, sizeof(*delete_on_success)); - char **subtable_locks = - reftable_calloc(compact_count + 1, sizeof(*subtable_locks)); + int compact_count; int i = 0; int j = 0; int is_empty_table = 0; @@ -989,6 +985,10 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, goto done; } + compact_count = last - first + 1; + REFTABLE_CALLOC_ARRAY(delete_on_success, compact_count + 1); + REFTABLE_CALLOC_ARRAY(subtable_locks, compact_count + 1); + st->stats.attempts++; strbuf_reset(&lock_file_name); @@ -1146,12 +1146,14 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, done: free_names(delete_on_success); - listp = subtable_locks; - while (*listp) { - unlink(*listp); - listp++; + if (subtable_locks) { + listp = subtable_locks; + while (*listp) { + unlink(*listp); + listp++; + } + free_names(subtable_locks); } - free_names(subtable_locks); if (lock_file_fd >= 0) { close(lock_file_fd); lock_file_fd = -1; From patchwork Thu Feb 1 07:33:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540716 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 2450E15A4A7 for ; Thu, 1 Feb 2024 07:33:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772792; cv=none; b=oU6pFtWUmyWU6XY1Z/Z5bZ9LXK2TVEd432nmnC16qiWUbsMlwrvMB795TRnllGnlBdR+Qvt16hW2NxUbTT0Wm4hBHcLK6iXIRmgg7x2voANGzv39L6FW5g8TQcOp0NrjladLIg3hOtH33Z3EKeH5SqzuHZkJQ6T8GiY3p9c6k5o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772792; c=relaxed/simple; bh=W07L0VM6Wkf2Baujko45vdHmD5zZ/3SW4KPU0oQPM7A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hIh1aaN1vh4P10MLkN83otpaqQWbXx1rPThesjxFuG4MS0ID5F9FRfjwwbii6MvXlI/g8hwP1dkux+ewLvAlRTsqKoHlvzkMwXJdl+QUixXOOR8tgKibt3NbxY3/qWQ8q6X/+31HUKTuOgzGiNtyaD8jVB3VdvIY+EYYK1f6I9s= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=lL+77xKj; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=IHPZ2anF; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="lL+77xKj"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="IHPZ2anF" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.west.internal (Postfix) with ESMTP id 4CA681800085; Thu, 1 Feb 2024 02:33:10 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 01 Feb 2024 02:33:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772789; x=1706859189; bh=HAdUdDM/Td 2dZ+w2m+b9txK8SuIsM3CjBRz/biOOUSI=; b=lL+77xKj8uNpbKdkCWjf09VH1/ R915YTzdS/f5RZ6FXpUeXwPGsZa1sAFhD4BMhtQDNwgPbZIQQrRFo4nLgEQaqQWH pDeeDV7M8Sndlao80XxUMsfY4PioupRSoY7RQEAExdc5j23K2n7qI9edKfSBzBrs pyDhFBFfuAFCCzHnliRhseCSHlK37nzRuiq9iwXE1V85LKPEyyX72wzWcQYRnYqj hpXwR20i6Zx9L1VcwMlFMdIGibDObzSrZjECXAQj6QMI+vqw0K319CTBTCw0i3HE ziIGRy/fb2ZTHoscunSCyaG+Fg2ON4GKTvwxtgHhlHsW5P/kkF4qyzg1AiWg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772789; x=1706859189; bh=HAdUdDM/Td2dZ+w2m+b9txK8SuIs M3CjBRz/biOOUSI=; b=IHPZ2anFqa2WrCUlJKXdp7IrrDz2giGuAjUkgzYJLL9l sS36C+3a2wXvq5gZvsjLSvf18xKinn3I/hq5z6xWdzChVSzT5+eLx3dhDS73Z+6a G7/Og3wg6t1gQ+72Y0DMq6Q7trXP7RDzkKDid2YjBG0QyTh6lx/1yHIY7wvKN1De 7gB3N3RV3adzt/U9a3dmzjwC5EY3C9pEIGdamcTYte3NM6F2wTFWJLljAgIRCfAP Zy6iPUAPakCfK9xtatBLpRQh/0YiHwllnBkqsAAKKz++te8pZGP3HcTyS0ZFN4/C XTmUofJ1Qo+kwrI0GrazKJRrchlE5WsP1i84GnWYPA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:08 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id cdeb2259 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:46 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:07 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 4/9] reftable/stack: index segments with `size_t` Message-ID: <50dac904e86830aadcd4d3890ef6a0567d501bcf.1706772591.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: We use `int`s to index into arrays of segments and track the length of them, which is considered to be a code smell in the Git project. Convert the code to use `size_t` instead. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 25 +++++++++++-------------- reftable/stack.h | 6 +++--- reftable/stack_test.c | 7 +++---- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index b6b24c90bf..2be3d1e4ba 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1202,12 +1202,11 @@ int fastlog2(uint64_t sz) return l - 1; } -struct segment *sizes_to_segments(int *seglen, uint64_t *sizes, int n) +struct segment *sizes_to_segments(size_t *seglen, uint64_t *sizes, size_t n) { struct segment *segs = reftable_calloc(n, sizeof(*segs)); - int next = 0; struct segment cur = { 0 }; - int i = 0; + size_t next = 0, i; if (n == 0) { *seglen = 0; @@ -1233,29 +1232,27 @@ struct segment *sizes_to_segments(int *seglen, uint64_t *sizes, int n) return segs; } -struct segment suggest_compaction_segment(uint64_t *sizes, int n) +struct segment suggest_compaction_segment(uint64_t *sizes, size_t n) { - int seglen = 0; - struct segment *segs = sizes_to_segments(&seglen, sizes, n); struct segment min_seg = { .log = 64, }; - int i = 0; + struct segment *segs; + size_t seglen = 0, i; + + segs = sizes_to_segments(&seglen, sizes, n); for (i = 0; i < seglen; i++) { - if (segment_size(&segs[i]) == 1) { + if (segment_size(&segs[i]) == 1) continue; - } - if (segs[i].log < min_seg.log) { + if (segs[i].log < min_seg.log) min_seg = segs[i]; - } } while (min_seg.start > 0) { - int prev = min_seg.start - 1; - if (fastlog2(min_seg.bytes) < fastlog2(sizes[prev])) { + size_t prev = min_seg.start - 1; + if (fastlog2(min_seg.bytes) < fastlog2(sizes[prev])) break; - } min_seg.start = prev; min_seg.bytes += sizes[prev]; diff --git a/reftable/stack.h b/reftable/stack.h index c1e3efa899..d919455669 100644 --- a/reftable/stack.h +++ b/reftable/stack.h @@ -32,13 +32,13 @@ struct reftable_stack { int read_lines(const char *filename, char ***lines); struct segment { - int start, end; + size_t start, end; int log; uint64_t bytes; }; int fastlog2(uint64_t sz); -struct segment *sizes_to_segments(int *seglen, uint64_t *sizes, int n); -struct segment suggest_compaction_segment(uint64_t *sizes, int n); +struct segment *sizes_to_segments(size_t *seglen, uint64_t *sizes, size_t n); +struct segment suggest_compaction_segment(uint64_t *sizes, size_t n); #endif diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 289e902146..2d5b24e5c5 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -711,7 +711,7 @@ static void test_sizes_to_segments(void) uint64_t sizes[] = { 2, 3, 4, 5, 7, 9 }; /* .................0 1 2 3 4 5 */ - int seglen = 0; + size_t seglen = 0; struct segment *segs = sizes_to_segments(&seglen, sizes, ARRAY_SIZE(sizes)); EXPECT(segs[2].log == 3); @@ -726,7 +726,7 @@ static void test_sizes_to_segments(void) static void test_sizes_to_segments_empty(void) { - int seglen = 0; + size_t seglen = 0; struct segment *segs = sizes_to_segments(&seglen, NULL, 0); EXPECT(seglen == 0); reftable_free(segs); @@ -735,8 +735,7 @@ static void test_sizes_to_segments_empty(void) static void test_sizes_to_segments_all_equal(void) { uint64_t sizes[] = { 5, 5 }; - - int seglen = 0; + size_t seglen = 0; struct segment *segs = sizes_to_segments(&seglen, sizes, ARRAY_SIZE(sizes)); EXPECT(seglen == 1); From patchwork Thu Feb 1 07:33:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540717 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 5859D15A4A7 for ; Thu, 1 Feb 2024 07:33:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772796; cv=none; b=YOejrXFNpcXvQzWB5CwzIfWYHL/qilTzHoXx8lZYGAqOFxdl8wzNzfVIfBY1mWty+zRNPmtTwJ8utjsEiRo/EPO72pfOgt+qxjkugZNpvqTJAXJxRpMdib86k3EAqxpPNyw06p1s7Lch/yKENkIjdVoEEAyuKi5f2CQXmi1rc2M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772796; c=relaxed/simple; bh=ruDhRxZc6F8BM6ntlIo/OFLidJLUQkESiFAA3ssUS5A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=B+oV8p8/NkOZVDZ3XylRewd/lgzo9wv5SX/nfuHFgqtesRbVDBAydhqwzUuDkPH+SVC0PP34zJ4OFb+YUk6vrQysgiurdNuwMuaGtLvZ25+kVy3hBNsPy1Hy7UdJTokgfLSH0wVuja+qvAC/KqQKfxzAG8o6OI64z2M3/ex3xFM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=Rh/sv6Zb; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=MzXZPZnO; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Rh/sv6Zb"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="MzXZPZnO" Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfhigh.west.internal (Postfix) with ESMTP id 6CC6F1800085; Thu, 1 Feb 2024 02:33:14 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 01 Feb 2024 02:33:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772793; x=1706859193; bh=UfhdZo0+dM Tvae4WJbNZZY1yLMuG3ZBzY42wlaRwnb8=; b=Rh/sv6Zb538FUeDoukasaN3aW8 EZxXAn2IofRGJvMb98nR5METoqT735RcHuEHywheb6w5E6eLkAwwlr2ZKSCzb7io Ne7GjA+KnaK+Lyd/e2QFLipmDw4rxke6y79wsI5eY4sZ0F+kbAYCRlr4HItGTYcm OLmjkufpKBRRhCCB6s2fxdL7igDL13LpkHWeqcIp7EJQQSHMbqQNgUTOC9w3oO2l SF2V3FPrUqpCr9HG6pnBRQKE82/vt/JdIEqwvnpl3X5S64arTkDMfyJFCOvsXqJf TR3dyvn66G0dLZFKH25gDArNjDUNROfFuDT4qmM53WLfGO23Eet+j4I52u8w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772793; x=1706859193; bh=UfhdZo0+dMTvae4WJbNZZY1yLMuG 3ZBzY42wlaRwnb8=; b=MzXZPZnOlqmJ7pfzp/qzfNppckVOPDXPB5JDGqJbC6Sy o8lsoyq6gpRwvp4Zf6UUgybfDVFzfm+t/B3ksu4smglkwFMjvt1sHIUkeCgkIbQo qE9CqOIvR9xifcAEwWYmrFeq938t0KcyTeSvspiSNUzbfPtgiled9gVUGMdEz9H4 1PVV6kpFPHQxFXXS+DPhX12W1L32DccUsnr8hIQdYBSrBh6UoHlVwRUvYm3pfTxr NLXJU9Eke3ZvXt8Pbd1agCKPO9pR8mJlECXwug2nhuD+Gl33jTXwId6CvuDxirBn V8vK5jnoMA3knjwzF0BRU98QD6nTAYvDx4gkyRkBXA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:12 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 864d3d73 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:50 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:11 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 5/9] reftable/stack: use `size_t` to track stack slices during compaction Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We use `int`s to track reftable slices when compacting the reftable stack, which is considered to be a code smell in the Git project. Convert the code to use `size_t` instead. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 2be3d1e4ba..c1f8cf1cef 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -24,7 +24,8 @@ static int stack_try_add(struct reftable_stack *st, void *arg), void *arg); static int stack_write_compact(struct reftable_stack *st, - struct reftable_writer *wr, int first, int last, + struct reftable_writer *wr, + size_t first, size_t last, struct reftable_log_expiry_config *config); static int stack_check_addition(struct reftable_stack *st, const char *new_tab_name); @@ -820,7 +821,8 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st) return 1; } -static int stack_compact_locked(struct reftable_stack *st, int first, int last, +static int stack_compact_locked(struct reftable_stack *st, + size_t first, size_t last, struct strbuf *temp_tab, struct reftable_log_expiry_config *config) { @@ -864,22 +866,21 @@ static int stack_compact_locked(struct reftable_stack *st, int first, int last, } static int stack_write_compact(struct reftable_stack *st, - struct reftable_writer *wr, int first, int last, + struct reftable_writer *wr, + size_t first, size_t last, struct reftable_log_expiry_config *config) { int subtabs_len = last - first + 1; struct reftable_table *subtabs = reftable_calloc( last - first + 1, sizeof(*subtabs)); struct reftable_merged_table *mt = NULL; - int err = 0; struct reftable_iterator it = { NULL }; struct reftable_ref_record ref = { NULL }; struct reftable_log_record log = { NULL }; - uint64_t entries = 0; + int err = 0; - int i = 0, j = 0; - for (i = first, j = 0; i <= last; i++) { + for (size_t i = first, j = 0; i <= last; i++) { struct reftable_reader *t = st->readers[i]; reftable_table_from_reader(&subtabs[j++], t); st->stats.bytes += t->size; @@ -963,7 +964,8 @@ static int stack_write_compact(struct reftable_stack *st, } /* < 0: error. 0 == OK, > 0 attempt failed; could retry. */ -static int stack_compact_range(struct reftable_stack *st, int first, int last, +static int stack_compact_range(struct reftable_stack *st, + size_t first, size_t last, struct reftable_log_expiry_config *expiry) { char **delete_on_success = NULL, **subtable_locks = NULL, **listp = NULL; @@ -972,12 +974,10 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, struct strbuf lock_file_name = STRBUF_INIT; struct strbuf ref_list_contents = STRBUF_INIT; struct strbuf new_table_path = STRBUF_INIT; + size_t i, j, compact_count; int err = 0; int have_lock = 0; int lock_file_fd = -1; - int compact_count; - int i = 0; - int j = 0; int is_empty_table = 0; if (first > last || (!expiry && first == last)) { @@ -1172,17 +1172,17 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, int reftable_stack_compact_all(struct reftable_stack *st, struct reftable_log_expiry_config *config) { - return stack_compact_range(st, 0, st->merged->stack_len - 1, config); + return stack_compact_range(st, 0, st->merged->stack_len ? + st->merged->stack_len - 1 : 0, config); } -static int stack_compact_range_stats(struct reftable_stack *st, int first, - int last, +static int stack_compact_range_stats(struct reftable_stack *st, + size_t first, size_t last, struct reftable_log_expiry_config *config) { int err = stack_compact_range(st, first, last, config); - if (err > 0) { + if (err > 0) st->stats.failures++; - } return err; } From patchwork Thu Feb 1 07:33:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540718 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 6C29715A4B0 for ; Thu, 1 Feb 2024 07:33:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772801; cv=none; b=MLWED2q1zJZjiADBWZ6uiL5v8ClM6Zh3pkTgRh8Bcm9yW9DquwroXB7+P5EeSQi9ze8IJLF3QOF1jAnULtY58VWfJGWDjNZMfDOB4sFTy+S3aTRZh0fplMhEE62fuq4GsQxYc7oMFjQUyECgFelOrORvEL3oiYs4chsh8py69M8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772801; c=relaxed/simple; bh=1hHg6Y1vuGvkBLSiwTUY8+9bJ1eTXl2qdVkgXLMFyL4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fhfJAmZglab20dLnb5+0d32/C3lCzikFHDU/VYPIIH41kV8GFm96X+ru2G6O2aPn1YLT4aKEH3WlNfWpMVp08QFsTrB1eeTbCU44FmIsYxdRQJoD3V56+ve/zBxDs6gK2X4DdP4X4UZzZfsJv6mi7PXlGL7/EKsdQlggLJtIItc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=LWV2Sve+; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=iCrUcgJo; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="LWV2Sve+"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="iCrUcgJo" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfhigh.west.internal (Postfix) with ESMTP id 989BD1800085; Thu, 1 Feb 2024 02:33:18 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 01 Feb 2024 02:33:18 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772798; x=1706859198; bh=m32ppqIUCY 0iwopfj/zwcdOLxOtpyTtRN5ErP5ZQxuU=; b=LWV2Sve+PdoEx7Yqv7OH7v6NRn Oar3UNeohMBDfO9dCzurhd2VFsJ8iNCoBaFupoFxxTwOQyR+L/iL6y24UAzauNRi QQAYE7kOu8vHEECDbhPkTeEsbjILdPYdyyVAvBhR58EtSi8HpyxogzCo0XHwe5Kk VP3U6+UO+8CHgFMzqQ3jW4xTG8U7WpS21s7Q22S5ZU3UggjtwyGJpKAzDyf9h5ER DoNuWZlFR1a8i41UrLbCxEOcHxBWLAzwTZIgyqA3yzdX/B9E5uhqA3rGBaV1X7Fl 3ltE/AANX/vKjX42rIFa7578y/IdYvHfUq3AA3e6Cj/hEFeSUUy2sJVorcwA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772798; x=1706859198; bh=m32ppqIUCY0iwopfj/zwcdOLxOtp yTtRN5ErP5ZQxuU=; b=iCrUcgJorzxJXhg4SyMPPd8Dtb/rcaMukHVr5X2QATnM Amlz9BJt9rG74RRh5opKX8aTKQuVGOtFMyDiLY5jz+hikDqkRboA8l5YZOPMYEzB SFUnPkclwDZwvIR1369hHw6JA0/IyKsFruBJ5/ZJbGGfgUZSgcxvsBFO6qJEvqUN YaqSqL6WFLJtfDiPbIxf8Xvq5AVFpsYUVkDsQZaoydE5PeAiJDv8SONx/C5GuLmc bRwd4jsgFZe1HNh7S01Qjd2N4XgW8edC2SM11qIkyqSHbxZGInLUD9RuEpyjJDw3 q2lr9WCe6WmadMEyvq8B/eQ4GhATx9Kb1df/7M7DgA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:17 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 49dec31a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:54 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:15 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 6/9] reftable/stack: use `size_t` to track stack length Message-ID: <55605fb53bfc0c81b9f2beec9c279df2634ebe4e.1706772591.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: While the stack length is already stored as `size_t`, we frequently use `int`s to refer to those stacks throughout the reftable library. Convert those cases to use `size_t` instead to make things consistent. Signed-off-by: Patrick Steinhardt --- reftable/basics.c | 7 +++---- reftable/basics.h | 2 +- reftable/merged.c | 11 +++++------ reftable/merged_test.c | 14 ++++++-------- reftable/reftable-merged.h | 2 +- reftable/stack.c | 21 ++++++++++----------- 6 files changed, 26 insertions(+), 31 deletions(-) diff --git a/reftable/basics.c b/reftable/basics.c index af9004cec2..0785aff941 100644 --- a/reftable/basics.c +++ b/reftable/basics.c @@ -64,12 +64,11 @@ void free_names(char **a) reftable_free(a); } -int names_length(char **names) +size_t names_length(char **names) { char **p = names; - for (; *p; p++) { - /* empty */ - } + while (*p) + p++; return p - names; } diff --git a/reftable/basics.h b/reftable/basics.h index 4c3ac963a3..91f3533efe 100644 --- a/reftable/basics.h +++ b/reftable/basics.h @@ -44,7 +44,7 @@ void parse_names(char *buf, int size, char ***namesp); int names_equal(char **a, char **b); /* returns the array size of a NULL-terminated array of strings. */ -int names_length(char **names); +size_t names_length(char **names); /* Allocation routines; they invoke the functions set through * reftable_set_alloc() */ diff --git a/reftable/merged.c b/reftable/merged.c index 2031fd51b4..e2c6253324 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -45,11 +45,10 @@ static int merged_iter_init(struct merged_iter *mi) static void merged_iter_close(void *p) { struct merged_iter *mi = p; - int i = 0; + merged_iter_pqueue_release(&mi->pq); - for (i = 0; i < mi->stack_len; i++) { + for (size_t i = 0; i < mi->stack_len; i++) reftable_iterator_destroy(&mi->stack[i]); - } reftable_free(mi->stack); strbuf_release(&mi->key); strbuf_release(&mi->entry_key); @@ -168,14 +167,14 @@ static void iterator_from_merged_iter(struct reftable_iterator *it, } int reftable_new_merged_table(struct reftable_merged_table **dest, - struct reftable_table *stack, int n, + struct reftable_table *stack, size_t n, uint32_t hash_id) { struct reftable_merged_table *m = NULL; uint64_t last_max = 0; uint64_t first_min = 0; - int i = 0; - for (i = 0; i < n; i++) { + + for (size_t i = 0; i < n; i++) { uint64_t min = reftable_table_min_update_index(&stack[i]); uint64_t max = reftable_table_max_update_index(&stack[i]); diff --git a/reftable/merged_test.c b/reftable/merged_test.c index e233a9d581..442917cc83 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -88,18 +88,17 @@ static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, struct reftable_reader ***readers, int *sizes, - struct strbuf *buf, int n) + struct strbuf *buf, size_t n) { - int i = 0; struct reftable_merged_table *mt = NULL; - int err; struct reftable_table *tabs; + int err; REFTABLE_CALLOC_ARRAY(tabs, n); REFTABLE_CALLOC_ARRAY(*readers, n); REFTABLE_CALLOC_ARRAY(*source, n); - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { write_test_table(&buf[i], refs[i], sizes[i]); block_source_from_strbuf(&(*source)[i], &buf[i]); @@ -263,18 +262,17 @@ static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, struct reftable_reader ***readers, int *sizes, - struct strbuf *buf, int n) + struct strbuf *buf, size_t n) { - int i = 0; struct reftable_merged_table *mt = NULL; - int err; struct reftable_table *tabs; + int err; REFTABLE_CALLOC_ARRAY(tabs, n); REFTABLE_CALLOC_ARRAY(*readers, n); REFTABLE_CALLOC_ARRAY(*source, n); - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { write_test_log_table(&buf[i], logs[i], sizes[i], i + 1); block_source_from_strbuf(&(*source)[i], &buf[i]); diff --git a/reftable/reftable-merged.h b/reftable/reftable-merged.h index 1a6d16915a..c91a2d83a2 100644 --- a/reftable/reftable-merged.h +++ b/reftable/reftable-merged.h @@ -33,7 +33,7 @@ struct reftable_table; the stack array. */ int reftable_new_merged_table(struct reftable_merged_table **dest, - struct reftable_table *stack, int n, + struct reftable_table *stack, size_t n, uint32_t hash_id); /* returns an iterator positioned just before 'name' */ diff --git a/reftable/stack.c b/reftable/stack.c index c1f8cf1cef..079ba7fde8 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -202,18 +202,18 @@ static struct reftable_reader **stack_copy_readers(struct reftable_stack *st, static int reftable_stack_reload_once(struct reftable_stack *st, char **names, int reuse_open) { - int cur_len = !st->merged ? 0 : st->merged->stack_len; + size_t cur_len = !st->merged ? 0 : st->merged->stack_len; struct reftable_reader **cur = stack_copy_readers(st, cur_len); - int err = 0; - int names_len = names_length(names); + size_t names_len = names_length(names); struct reftable_reader **new_readers = reftable_calloc(names_len, sizeof(*new_readers)); struct reftable_table *new_tables = reftable_calloc(names_len, sizeof(*new_tables)); - int new_readers_len = 0; + size_t new_readers_len = 0; struct reftable_merged_table *new_merged = NULL; struct strbuf table_path = STRBUF_INIT; - int i; + int err = 0; + size_t i; while (*names) { struct reftable_reader *rd = NULL; @@ -221,11 +221,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, /* this is linear; we assume compaction keeps the number of tables under control so this is not quadratic. */ - int j = 0; - for (j = 0; reuse_open && j < cur_len; j++) { - if (cur[j] && 0 == strcmp(cur[j]->name, name)) { - rd = cur[j]; - cur[j] = NULL; + for (i = 0; reuse_open && i < cur_len; i++) { + if (cur[i] && 0 == strcmp(cur[i]->name, name)) { + rd = cur[i]; + cur[i] = NULL; break; } } @@ -870,7 +869,7 @@ static int stack_write_compact(struct reftable_stack *st, size_t first, size_t last, struct reftable_log_expiry_config *config) { - int subtabs_len = last - first + 1; + size_t subtabs_len = last - first + 1; struct reftable_table *subtabs = reftable_calloc( last - first + 1, sizeof(*subtabs)); struct reftable_merged_table *mt = NULL; From patchwork Thu Feb 1 07:33:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540719 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 16AB7158D81 for ; Thu, 1 Feb 2024 07:33:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772805; cv=none; b=Gtsmaw3cu01Fz9yrK8eizF729UDPcQ6tncYxxgo7+8HjRhTo+BTQd6rRqF39RrGvoTEbcw2xpek8w+Ct+28Y8a8JLaIIU9i2L6D0u+O3bwy1OPGqTka7k+vtGPOUjLJy+gOFjyyEQCJuSXHLBqoZwZeoYVNdpCzZBa3SRNS7tSo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772805; c=relaxed/simple; bh=d0ivr+yGCD+kEvcVYb7HVRYZlfabGqrez78Gz0ggHMA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=L/veC6BJg+hqBOUEU3VrCQQhJ+CGQL/DFzOYyUG2OlphBQoW6YB38+aUGZVdKkZ8cOmg6+qARhh383cFrm8CqFzHJgBTKYOY8no+X5b6qtgdM+59FpiUUxxY7bkWMWj0NmoJhAbEX2fbgjnUKRD6+Arv4iPYSz6Cbh0q17i05SI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=Tc/rdq9a; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=rBnuiPGB; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Tc/rdq9a"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="rBnuiPGB" Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailfhigh.west.internal (Postfix) with ESMTP id 12A2F1800085; Thu, 1 Feb 2024 02:33:22 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Thu, 01 Feb 2024 02:33:23 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772802; x=1706859202; bh=gau0Ngv/9d rp7CToT9JVGgDIK8w9nuUAmrETk0AwDEc=; b=Tc/rdq9aiFiOOGXcuPiGyH676e bgpZU8IvuDlKQxijjmp9sAhqE4xh2787hzu1eyIADjTsjIM5+ahvc0ICaS6ib3ST F1o+KMYkHcJgbiX56EYLTBhFOBRp8YHtE9Kbi8NP7U4k6yhjUmbX5kzzoI3JwTmL gdTZwbeSVGVJmcmfNBUcxA+uu6RcLSVFrkSaFyNHhCaxr2YNSx7QCC7aF/Kqyms1 QhrSoF5qYV1OIY6sGqT4Hzzs+Fno+HpEJjrJQqvyXRGmvjAz+/vFO/4gRLms8tCK U0iQKV1E167uRTX9RyZLVKsZM0m7S6TGkBUa/GffeTeOrONkdwa7yIpBjNPg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772802; x=1706859202; bh=gau0Ngv/9drp7CToT9JVGgDIK8w9 nuUAmrETk0AwDEc=; b=rBnuiPGB0ALAzEe5UvmcL21dqxum2+MFg6hZBt4AXNX1 MN/MS3V0v79H5fIz/vUSrFS07f/lkIiwKNrx2YMar12JH/DbdULGKFjMEG1NQpFE DW3lYmPitAgcdJ2MxMvlE9x+xwFFMD8rKfqETHFczrzHbz5P/4g4vaOrd1wYgbkB F39B28lJE+XPRAAGlT507WOOD7Q2ng/ROJlNaj/8jvwEcnpG0zGuSkGCp0DlI5eD D5Qp7C0euzSe2IzeQoAaGAi+1PkquTbVf1JJlrjSFwCgos4U3kdygx+WJrQFpjlM DCCKSMIe6WyEueUEQXz5l6dlAF/Dw54qTUNhFW+OaA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:21 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e9b1e6f4 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:29:58 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:19 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 7/9] reftable/merged: refactor seeking of records Message-ID: <80cf2fd272577b13f4fd23f009d22294629f5f3b.1706772591.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 to seek reftable records in the merged table code is quite hard to read and does not conform to our coding style in multiple ways: - We have multiple exit paths where we release resources even though that is not really necessary. - We use a scoped error variable `e` which is hard to reason about. This variable is not required at all. - We allocate memory in the variable declarations, which is easy to miss. Refactor the function so that it becomes more maintainable in the future. Signed-off-by: Patrick Steinhardt --- reftable/merged.c | 54 ++++++++++++++++++----------------------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index e2c6253324..0abcda26e8 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -238,50 +238,38 @@ static int merged_table_seek_record(struct reftable_merged_table *mt, struct reftable_iterator *it, struct reftable_record *rec) { - struct reftable_iterator *iters = reftable_calloc( - mt->stack_len, sizeof(*iters)); struct merged_iter merged = { - .stack = iters, .typ = reftable_record_type(rec), .hash_id = mt->hash_id, .suppress_deletions = mt->suppress_deletions, .key = STRBUF_INIT, .entry_key = STRBUF_INIT, }; - int n = 0; - int err = 0; - int i = 0; - for (i = 0; i < mt->stack_len && err == 0; i++) { - int e = reftable_table_seek_record(&mt->stack[i], &iters[n], - rec); - if (e < 0) { - err = e; - } - if (e == 0) { - n++; - } - } - if (err < 0) { - int i = 0; - for (i = 0; i < n; i++) { - reftable_iterator_destroy(&iters[i]); - } - reftable_free(iters); - return err; + struct merged_iter *p; + int err; + + REFTABLE_CALLOC_ARRAY(merged.stack, mt->stack_len); + for (size_t i = 0; i < mt->stack_len; i++) { + err = reftable_table_seek_record(&mt->stack[i], + &merged.stack[merged.stack_len], rec); + if (err < 0) + goto out; + if (!err) + merged.stack_len++; } - merged.stack_len = n; err = merged_iter_init(&merged); - if (err < 0) { + if (err < 0) + goto out; + + p = reftable_malloc(sizeof(struct merged_iter)); + *p = merged; + iterator_from_merged_iter(it, p); + +out: + if (err < 0) merged_iter_close(&merged); - return err; - } else { - struct merged_iter *p = - reftable_malloc(sizeof(struct merged_iter)); - *p = merged; - iterator_from_merged_iter(it, p); - } - return 0; + return err; } int reftable_merged_table_seek_ref(struct reftable_merged_table *mt, From patchwork Thu Feb 1 07:33:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540720 Received: from wfout2-smtp.messagingengine.com (wfout2-smtp.messagingengine.com [64.147.123.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 227FA15AADA for ; Thu, 1 Feb 2024 07:33:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772808; cv=none; b=ostp01f3rLuyvkrXKcxC/fwzUr3QPKoqF09T16JbsQJ02g7KBW2ZxM+WlTsngOd5cjEjh0N8p0VPXubuJdrJFYNfVDpKTwiCE15Ouvyh9jkBPAznXSBrZP9O93OhyUj5fTBsZRCyuiqYyeD4H4Nu+a/0Itmo/RFET6MKjH7desY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772808; c=relaxed/simple; bh=J/yK1aPIcEnEY/LhFO5/jHiTEXqnpGWpdcEAzN793S4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kqhaNEvuIOJsekC1a/++k3zuHpF3bR92fZupcveoovUsf5MVxwBrECwFeIJNMMwYrzfmsXOSlAFXNL9jdJJ+tbaDQULYaBJBwxcvWuoVKJDRlbcmk86rz2Mf6ZilblnoDyR+WzGuVAVZ2Ne6RF4josYgmoS05AOoFvgHq2WJr6E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=U2TsxuTE; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Wj7zbbA0; arc=none smtp.client-ip=64.147.123.145 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="U2TsxuTE"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Wj7zbbA0" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfout.west.internal (Postfix) with ESMTP id 4754E1C0007D; Thu, 1 Feb 2024 02:33:26 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Thu, 01 Feb 2024 02:33:26 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772805; x=1706859205; bh=1VOQR6ox9W o77Oi/3L5T7VKQXxzAqoyA1gj2ti5kNrk=; b=U2TsxuTETBxGS5PpGOayXQ+xae Zg72CiSWUcuHFg0K6MGXycF3KIeHrDESFScVb5cGNWw2vYvfrH7FjAeFFWf3PYws nQOD7Dy5WyojODs6pJ6mJ5dXgYca070p+nmqe5YEbzE8c3xVcadIcDUd3yL09dBb 7IeqJuQZPstM8XK1U73FS9FMqs2QhJeKDqjYs61V1Z/enOvPJuAklbLXy1PI7w5g RQW6ykRwNt4yFqTV64qvxjculk8Mm3q/NAl/c0fLZHWQUbLKCyK/qsaElo7jIlbl d3/TcJhVgG7UZkoVUGGdsZhOZmyw8+Kijvyku8LVcUexEaVl2mudURTR76WQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772805; x=1706859205; bh=1VOQR6ox9Wo77Oi/3L5T7VKQXxzA qoyA1gj2ti5kNrk=; b=Wj7zbbA0TYMYzOpCVZ4VSD76sqnOVd+j5CHdcb4uwFx1 BkQauhT13+bvfb3fWviugQDGww+Xb+Kdp0MhuAghm1Ec+HbJv5BFC/lAvxxPEzi0 nwVjAShfIUdcoq4bKhzeEJF2JGE2bym+pgSFVS6AOHR6U9BJoYpVGtMTphWHjMnt /WzYP5fx3q2ZgoSa4uCha9BJBeo1Z4THbQImIUQZUUoWZHgpNxPMXW32oKRn9wvH nBi9T0r2CJo9p6aaFsNW10KlMEihCZJcDWbmCYqts9Jd1CgGmPG+XLvqNtByZGZR mxX9kff6GZdbg1+rG1QcsdhbVrKi9dn2IpqITsAcVA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:24 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e3d6631e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:30:02 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:23 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 8/9] reftable/merged: refactor initialization of iterators Message-ID: <8c1be2b1599011f57479249c19f4e4742b7d8d8b.1706772591.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: Refactor the initialization of the merged iterator to fit our code style better. This refactoring prepares the code for a refactoring of how records are being initialized. Signed-off-by: Patrick Steinhardt --- reftable/merged.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index 0abcda26e8..0e60e2a39b 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -19,24 +19,23 @@ license that can be found in the LICENSE file or at static int merged_iter_init(struct merged_iter *mi) { - int i = 0; - for (i = 0; i < mi->stack_len; i++) { - struct reftable_record rec = reftable_new_record(mi->typ); - int err = iterator_next(&mi->stack[i], &rec); - if (err < 0) { + for (size_t i = 0; i < mi->stack_len; i++) { + struct pq_entry e = { + .rec = reftable_new_record(mi->typ), + .index = i, + }; + int err; + + err = iterator_next(&mi->stack[i], &e.rec); + if (err < 0) return err; - } - if (err > 0) { reftable_iterator_destroy(&mi->stack[i]); - reftable_record_release(&rec); - } else { - struct pq_entry e = { - .rec = rec, - .index = i, - }; - merged_iter_pqueue_add(&mi->pq, &e); + reftable_record_release(&e.rec); + continue; } + + merged_iter_pqueue_add(&mi->pq, &e); } return 0; From patchwork Thu Feb 1 07:33:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13540721 Received: from wfhigh3-smtp.messagingengine.com (wfhigh3-smtp.messagingengine.com [64.147.123.154]) (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 71F2615AAAB for ; Thu, 1 Feb 2024 07:33:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772813; cv=none; b=rQGFS6py/VvW85YCwQ7XHbRvw/OPoMJ0KVwum4REzMyxNJhGCPpqqk6jpWfXceijbHda2/lcZbQzoYpQ3++ilH0xD2wFRlHfXRSIEHh2DO3xvr0U5qF0rWN7n/XiBX1IP84PFaqss+cQdlMlSQZzy8neNX3UHdJgaxzmFkzFQiY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706772813; c=relaxed/simple; bh=bwtbeDCOm7PgYtXtbdzDYFvX/31delFcBzapTGBjUoo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ABzBXMi/q8QxOXlHMdVxlz+ObxXFA7VaxDLixXYD2PSu1TUVC1ys0YJugxv6S271c8BGtvRmJa/IdoS3af5PY0DPPf1ij92XfpUcQmaEWpZcVyvw3LVoGLuPQ3gxY2jlpaASa/Tlms5jBTLDHbSbBa1ku0Z0oryGuFF2jf+aitk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=qGsVkBYU; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=U5bxzaTX; arc=none smtp.client-ip=64.147.123.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="qGsVkBYU"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="U5bxzaTX" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.west.internal (Postfix) with ESMTP id 715751800085; Thu, 1 Feb 2024 02:33:30 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Thu, 01 Feb 2024 02:33:30 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1706772809; x=1706859209; bh=pPoBr4V97R kdySAxGMEoE6061eddqJnta7Pyy4MnXnE=; b=qGsVkBYUrvf1V65i9qWtknr1sw juapT6dOkKw699zFLfd/0rWPghGj+bsjMklSPfyqW9qcuOidJ+ORbGiTy/YJk2Qe Z8AP4El/ZeXXIgaS+V6VjTj8jgMpYH4otNGa0dIix6vBNv/be25Vu1zh4uJWmT8/ r4VvvJQJvqzfMuoyr6MUqWvlcNvAyKkAoEOyUfxybUcHqW1x7ftz9aITyTCQKL++ urJJxCjGpVCOr4uE4FizPB8uJBKyn+XchNcEM8j8aCsSD28r6I0EmY3y6COXT4hP SqBPgBZGrLJhXWdLrgAiGYnYbMfI2LmwMtOHyFczf7OY0RfiHlol41XyXebA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1706772809; x=1706859209; bh=pPoBr4V97RkdySAxGMEoE6061edd qJnta7Pyy4MnXnE=; b=U5bxzaTXIKkecQeRmyVuLNS5leNSXvYGBGhAjMAyy3Nc K0p73EqTlM6hwgkpPRMyRm861OdO5tbPFfxyJ1/PIFdREorFe+Vfh0eRR7nY+UAp CDApfjcJiLsuOOtwyXAjK+ajeZdHYVgENy27QaykDFiM/9PN2U1nMnBm0YdOamzz fhHQ8qcUszQISHHe5JyNan/7WS2YvLTLNkUJTmOHHY9csmCi73CVIcI9POqbBlUG jSlkMna1OqoMSvH6oPAvxOQiBmjVOmgvt51NPhLjQTitG0Cw7zXPUgWo/uhCMzdJ 3WqOuMKFw1AlVt8gcvmKdfg/W96Pfmy3VIlqG4/AyA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedutddguddtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Feb 2024 02:33:28 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e2e4e5af (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Feb 2024 07:30:06 +0000 (UTC) Date: Thu, 1 Feb 2024 08:33:27 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Eric Sunshine , Junio C Hamano Subject: [PATCH v2 9/9] reftable/record: improve semantics when initializing records 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: According to our usual coding style, the `reftable_new_record()` function would indicate that it is allocating a new record. This is not the case though as the function merely initializes records without allocating any memory. Replace `reftable_new_record()` with a new `reftable_record_init()` function that takes a record pointer as input and initializes it accordingly. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 18 +++++++++--------- reftable/merged.c | 8 +++++--- reftable/reader.c | 4 ++-- reftable/record.c | 43 ++++++++++-------------------------------- reftable/record.h | 10 +++++----- reftable/record_test.c | 4 ++-- 6 files changed, 33 insertions(+), 54 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 6952d0facf..9ad220747e 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -380,23 +380,23 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, .key = *want, .r = br, }; - struct reftable_record rec = reftable_new_record(block_reader_type(br)); - int err = 0; struct block_iter next = BLOCK_ITER_INIT; + struct reftable_record rec; + int err = 0, i; - int i = binsearch(br->restart_count, &restart_key_less, &args); if (args.error) { err = REFTABLE_FORMAT_ERROR; goto done; } - it->br = br; - if (i > 0) { - i--; - it->next_off = block_reader_restart_offset(br, i); - } else { + i = binsearch(br->restart_count, &restart_key_less, &args); + if (i > 0) + it->next_off = block_reader_restart_offset(br, i - 1); + else it->next_off = br->header_off + 4; - } + it->br = br; + + reftable_record_init(&rec, block_reader_type(br)); /* We're looking for the last entry less/equal than the wanted key, so we have to go one entry too far and then back up. diff --git a/reftable/merged.c b/reftable/merged.c index 0e60e2a39b..a0f222e07b 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -21,11 +21,11 @@ static int merged_iter_init(struct merged_iter *mi) { for (size_t i = 0; i < mi->stack_len; i++) { struct pq_entry e = { - .rec = reftable_new_record(mi->typ), .index = i, }; int err; + reftable_record_init(&e.rec, mi->typ); err = iterator_next(&mi->stack[i], &e.rec); if (err < 0) return err; @@ -57,10 +57,12 @@ static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi, size_t idx) { struct pq_entry e = { - .rec = reftable_new_record(mi->typ), .index = idx, }; - int err = iterator_next(&mi->stack[idx], &e.rec); + int err; + + reftable_record_init(&e.rec, mi->typ); + err = iterator_next(&mi->stack[idx], &e.rec); if (err < 0) return err; diff --git a/reftable/reader.c b/reftable/reader.c index 3e0de5e8ad..5e6c8f30a1 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -444,13 +444,13 @@ static int reader_start(struct reftable_reader *r, struct table_iter *ti, static int reader_seek_linear(struct table_iter *ti, struct reftable_record *want) { - struct reftable_record rec = - reftable_new_record(reftable_record_type(want)); struct strbuf want_key = STRBUF_INIT; struct strbuf got_key = STRBUF_INIT; struct table_iter next = TABLE_ITER_INIT; + struct reftable_record rec; int err = -1; + reftable_record_init(&rec, reftable_record_type(want)); reftable_record_key(want, &want_key); while (1) { diff --git a/reftable/record.c b/reftable/record.c index 5c3fbb7b2a..8b84b8157f 100644 --- a/reftable/record.c +++ b/reftable/record.c @@ -1257,45 +1257,22 @@ reftable_record_vtable(struct reftable_record *rec) abort(); } -struct reftable_record reftable_new_record(uint8_t typ) +void reftable_record_init(struct reftable_record *rec, uint8_t typ) { - struct reftable_record clean = { - .type = typ, - }; + memset(rec, 0, sizeof(*rec)); + rec->type = typ; - /* the following is involved, but the naive solution (just return - * `clean` as is, except for BLOCK_TYPE_INDEX), returns a garbage - * clean.u.obj.offsets pointer on Windows VS CI. Go figure. - */ switch (typ) { - case BLOCK_TYPE_OBJ: - { - struct reftable_obj_record obj = { 0 }; - clean.u.obj = obj; - break; - } - case BLOCK_TYPE_INDEX: - { - struct reftable_index_record idx = { - .last_key = STRBUF_INIT, - }; - clean.u.idx = idx; - break; - } case BLOCK_TYPE_REF: - { - struct reftable_ref_record ref = { 0 }; - clean.u.ref = ref; - break; - } case BLOCK_TYPE_LOG: - { - struct reftable_log_record log = { 0 }; - clean.u.log = log; - break; - } + case BLOCK_TYPE_OBJ: + return; + case BLOCK_TYPE_INDEX: + strbuf_init(&rec->u.idx.last_key, 0); + return; + default: + BUG("unhandled record type"); } - return clean; } void reftable_record_print(struct reftable_record *rec, int hash_size) diff --git a/reftable/record.h b/reftable/record.h index fd80cd451d..e64ed30c80 100644 --- a/reftable/record.h +++ b/reftable/record.h @@ -69,9 +69,6 @@ struct reftable_record_vtable { /* returns true for recognized block types. Block start with the block type. */ int reftable_is_block_type(uint8_t typ); -/* return an initialized record for the given type */ -struct reftable_record reftable_new_record(uint8_t typ); - /* Encode `key` into `dest`. Sets `is_restart` to indicate a restart. Returns * number of bytes written. */ int reftable_encode_key(int *is_restart, struct string_view dest, @@ -100,8 +97,8 @@ struct reftable_obj_record { /* record is a generic wrapper for different types of records. It is normally * created on the stack, or embedded within another struct. If the type is * known, a fresh instance can be initialized explicitly. Otherwise, use - * reftable_new_record() to initialize generically (as the index_record is not - * valid as 0-initialized structure) + * `reftable_record_init()` to initialize generically (as the index_record is + * not valid as 0-initialized structure) */ struct reftable_record { uint8_t type; @@ -113,6 +110,9 @@ struct reftable_record { } u; }; +/* Initialize the reftable record for the given type */ +void reftable_record_init(struct reftable_record *rec, uint8_t typ); + /* see struct record_vtable */ int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size); void reftable_record_print(struct reftable_record *rec, int hash_size); diff --git a/reftable/record_test.c b/reftable/record_test.c index 999366ad46..a86cff5526 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -16,11 +16,11 @@ static void test_copy(struct reftable_record *rec) { - struct reftable_record copy = { 0 }; + struct reftable_record copy; uint8_t typ; typ = reftable_record_type(rec); - copy = reftable_new_record(typ); + reftable_record_init(©, typ); reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); /* do it twice to catch memory leaks */ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ);