From patchwork Tue Jun 4 10:13:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13684996 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 116CC143C72 for ; Tue, 4 Jun 2024 10:13:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495988; cv=none; b=RxaHzFuW+z/9rtF42cviT0rspP0dYkSNiF4MG1I/DXK+Lk6wVIb+/pgDCiCpHYkxBKF2OMCmY1aVFK3B2Ux4e1S0GsHQ8/R3E/uPbVrAsdY+p64fYye3Rc6b/x3E1JDACm4jZAx/gwXRVABcPiTV2+zXJWPb1rFMNN9KK4lGAVs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495988; c=relaxed/simple; bh=Su3xFihH12BGsb+BCs1A3A2YWR85Ia/rvF8RsiNk8y4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=r9192OGvxRzt8RIw1NIeuIXh484wObWy+m4WaAogJaX85M6LN0p+asH+Bk0ydeeGdK6wHktE25DDJkrJn3JvVMYg3Hb0e0DicT3W5r2xWACU8aIaauIeQQ+Nm8FZ5uf+XsPe0iVCvq6itOxnIbQmMTqZMXLn6DGpH8kwuXBNS04= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21453 invoked by uid 109); 4 Jun 2024 10:13:06 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:06 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18342 invoked by uid 111); 4 Jun 2024 10:13:03 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:03 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:05 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 01/13] sparse-checkout: free string list in write_cone_to_file() Message-ID: <20240604101305.GA1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> We use a string list to hold sorted and de-duped patterns, but don't free it before leaving the function, causing a leak. This drops the number of leaks found in t7002 from 27 to 25. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 0f52e25249..8747191484 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -311,6 +311,8 @@ static void write_cone_to_file(FILE *fp, struct pattern_list *pl) fprintf(fp, "%s/\n", pattern); free(pattern); } + + string_list_clear(&sl, 0); } static int write_patterns_and_update(struct pattern_list *pl) From patchwork Tue Jun 4 10:13:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13684997 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 68CDA143C72 for ; Tue, 4 Jun 2024 10:13:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495994; cv=none; b=TqMpKucXo5OGQeB79a+Lc/af8VCpAv+zjrHEEf4lNEY8RLcFc9SwRQ/B+xqwimFTj2wqEzEHZlTKxbQZWoKcSBrySx5UYxo9SXcR7YeHUB41/cSY8d/xFwJGQyzpPIx+S5XeWyy+smM7fxHJrbgKqWZ2FPax7A5/U0FwKFM3tic= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495994; c=relaxed/simple; bh=RlSNqG5WYDP0W558P5TgjQ/VJZKuSuJaWrX19lH8QIw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BMBYk6AvEJN5TxWDe5mP/BwYXA1CrgI+VvGoLCE0QSD35DovBPKMzGuZv+ueZhmHZyoP/KrSliwoaOQ7dV+aerndOtG3dHovN3qNuVQhUIDmCCvX7g6hmOKzxEErk4LUv5j7KY5UrfgJfKtM2F14ujw8P4jZdWDvLpS6uriwJ2E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21463 invoked by uid 109); 4 Jun 2024 10:13:12 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:12 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18350 invoked by uid 111); 4 Jun 2024 10:13:08 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:08 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:10 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 02/13] sparse-checkout: pass string literals directly to add_pattern() Message-ID: <20240604101310.GB1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> The add_pattern() function takes a pattern string, but neither makes a copy of it nor takes ownership of the memory. So it is the caller's responsibility to make sure the string hangs around as long as the pattern_list which references it. There are a few cases in sparse-checkout where we use string literal patterns by stuffing them into a strbuf, detaching the buffer, and then passing the result into add_pattern(). This creates a leak when the pattern_list is eventually cleared, since we don't retain a copy of the detached buffer to free. But we can observe that the whole strbuf dance is unnecessary. The point was presumably[1] to satisfy the lifetime requirement of the string. But string literals have static duration; we can count on them lasting for the whole program. So we can fix the leak by just passing them directly. And as a bonus, that simplifies the code. The leaks can be seen in t7002, which drops from 25 leaks to 22 with this patch. It also makes t3602 and t1090 leak-free. In the long run, we will also want to clean up this (undocumented!) memory lifetime requirement of add_pattern(). But that can come in a later patch; passing the string literals directly will be the right thing either way. [1] The code in question comes from 416adc8711 (sparse-checkout: update working directory in-process for 'init', 2019-11-21) and 99dfa6f970 (sparse-checkout: use in-process update for disable subcommand, 2019-11-21), but I didn't see anything in their commit messages or on the list explaining the strbufs. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 11 +++-------- t/t1090-sparse-checkout-scope.sh | 1 + t/t3602-rm-sparse-checkout.sh | 1 + 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 8747191484..7c17ed238c 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -442,7 +442,6 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix) char *sparse_filename; int res; struct object_id oid; - struct strbuf pattern = STRBUF_INIT; static struct option builtin_sparse_checkout_init_options[] = { OPT_BOOL(0, "cone", &init_opts.cone_mode, @@ -493,10 +492,8 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix) return 0; } - strbuf_addstr(&pattern, "/*"); - add_pattern(strbuf_detach(&pattern, NULL), empty_base, 0, &pl, 0); - strbuf_addstr(&pattern, "!/*/"); - add_pattern(strbuf_detach(&pattern, NULL), empty_base, 0, &pl, 0); + add_pattern("/*", empty_base, 0, &pl, 0); + add_pattern("!/*/", empty_base, 0, &pl, 0); pl.use_cone_patterns = init_opts.cone_mode; return write_patterns_and_update(&pl); @@ -893,7 +890,6 @@ static int sparse_checkout_disable(int argc, const char **argv, OPT_END(), }; struct pattern_list pl; - struct strbuf match_all = STRBUF_INIT; /* * We do not exit early if !core_apply_sparse_checkout; due to the @@ -919,8 +915,7 @@ static int sparse_checkout_disable(int argc, const char **argv, pl.use_cone_patterns = 0; core_apply_sparse_checkout = 1; - strbuf_addstr(&match_all, "/*"); - add_pattern(strbuf_detach(&match_all, NULL), empty_base, 0, &pl, 0); + add_pattern("/*", empty_base, 0, &pl, 0); prepare_repo_settings(the_repository); the_repository->settings.sparse_index = 0; diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh index 3a14218b24..da0e7714d5 100755 --- a/t/t1090-sparse-checkout-scope.sh +++ b/t/t1090-sparse-checkout-scope.sh @@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME TEST_CREATE_REPO_NO_TEMPLATE=1 +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t3602-rm-sparse-checkout.sh b/t/t3602-rm-sparse-checkout.sh index 08580fd3dc..fcdefba48c 100755 --- a/t/t3602-rm-sparse-checkout.sh +++ b/t/t3602-rm-sparse-checkout.sh @@ -2,6 +2,7 @@ test_description='git rm in sparse checked out working trees' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' " From patchwork Tue Jun 4 10:13:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13684998 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 AC7E01448E2 for ; Tue, 4 Jun 2024 10:13:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495997; cv=none; b=ppFguJG8LiTVQM1XF5CAy57nzgXf5IKBefoQVJSLJtYqwKXwJgG0ynRzekJ6reP8MfQBSanBiXrf8vFD+ONEu3jVyZQmyAOlx0ZKdHsUtcgIt5Y0yTvqEgsyKM7TWpwUB3WdE//tBMPEOnrr64VDOjUCYWzMCWMur96jo2ANLFg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717495997; c=relaxed/simple; bh=IZOFFgG3RuI7Q+u5Wwn+mpRlJQANH8yw5GLOZwmaiWs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=etaVIBW2wXWf+iJIzwZCfzFvvhQOVZ2Ro+uGE7zZQl5HM39C2Qp8Swi4jmuzWLdqWCf1pC5gmrSTe1gU73dotL05Pe9rbRZQTiIJq0WkJmVQB/5iaQi+yje7HQnHYFI4/7EFF/9jwXJO1i9PQLObOpWjoWX+El/6f+HB6huxNJE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21475 invoked by uid 109); 4 Jun 2024 10:13:15 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:15 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18355 invoked by uid 111); 4 Jun 2024 10:13:12 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:12 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:14 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 03/13] dir.c: free strings in sparse cone pattern hashmaps Message-ID: <20240604101314.GC1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> The pattern_list structs used for cone-mode sparse lookups use a few extra hashmaps. These store pattern_entry structs, each of which has its own heap-allocated pattern string. When we clean up the hashmaps, we free the individual pattern_entry structs, but forget to clean up the embedded strings, causing memory leaks. We can fix this by iterating over the hashmaps to free the extra strings. This reduces the numbers of leaks in t7002 from 22 to 9. One alternative here would be to make the string a FLEX_ARRAY member of the pattern_entry. Then there's no extra free() required, and as a bonus it would be a little more efficient. However, some of the refactoring gets awkward, as we are often assigning strings allocated by helper functions. So let's just fix the leak for now, and we can explore bigger refactoring separately. Signed-off-by: Jeff King --- dir.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/dir.c b/dir.c index 2d83f3311a..9cc4a68fbc 100644 --- a/dir.c +++ b/dir.c @@ -726,6 +726,17 @@ static char *dup_and_filter_pattern(const char *pattern) return result; } +static void clear_pattern_entry_hashmap(struct hashmap *map) +{ + struct hashmap_iter iter; + struct pattern_entry *entry; + + hashmap_for_each_entry(map, &iter, entry, ent) { + free(entry->pattern); + } + hashmap_clear_and_free(map, struct pattern_entry, ent); +} + static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern *given) { struct pattern_entry *translated; @@ -855,8 +866,8 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern clear_hashmaps: warning(_("disabling cone pattern matching")); - hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent); - hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent); + clear_pattern_entry_hashmap(&pl->recursive_hashmap); + clear_pattern_entry_hashmap(&pl->parent_hashmap); pl->use_cone_patterns = 0; } @@ -956,8 +967,8 @@ void clear_pattern_list(struct pattern_list *pl) free(pl->patterns[i]); free(pl->patterns); free(pl->filebuf); - hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent); - hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent); + clear_pattern_entry_hashmap(&pl->recursive_hashmap); + clear_pattern_entry_hashmap(&pl->parent_hashmap); memset(pl, 0, sizeof(*pl)); } From patchwork Tue Jun 4 10:13:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13684999 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 EF8E5143C7B for ; Tue, 4 Jun 2024 10:13:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496000; cv=none; b=evGx0dFUxSAawsszTXCcYETfPRv0cXnhexB2ASY5/EYllojiQkj+gv/KoQRB4xd5iJbR8h34byVKfkyjVbyxBXI/rtTRyRTiK7knLomBrEscEYxXhNZFRn2C6F2X3cBfA5SciBf/qaBK/Nd14mbXjuGEj0X7BL0EdiPsL4fJGyM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496000; c=relaxed/simple; bh=rQC/gO4q1pb8NVDNGu86dTI/43gncmPvUr4FpiqNeMI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=W66HsTOuAPdPNwkRwXtO4URnJRTiqk3W+Ju28qPxZ+wHgivjYRVwYVOfwQIp3ND15FlWmYtxKXz1QlQkvvo6JMIXYfdUYwpEwIu35pW6HuJ4dmy3LElJP0n9A2jhtX1+oik0CVKouUgekn2iVCSj10sHr8wT8ChFcbulOevO/48= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21484 invoked by uid 109); 4 Jun 2024 10:13:18 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:18 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18360 invoked by uid 111); 4 Jun 2024 10:13:15 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:15 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:17 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 04/13] sparse-checkout: clear patterns when init() sees existing sparse file Message-ID: <20240604101317.GD1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In sparse_checkout_init(), we first try to load patterns from an existing file. If we found any, we return immediately, but end up leaking the patterns we parsed. Fixing this reduces the number of leaks in t7002 from 9 down to 5. Note that there are two other exits from the function, but they don't need the same treatment: - if we can't resolve HEAD, we write out a hard-coded sparse file and return. But we know the pattern list is empty there, since we didn't find any in the on-disk file and we haven't yet added any of our own. - otherwise, we do populate the list and then tail-call into write_patterns_and_update(). But that function frees the pattern_list itself, so we don't need to. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 7c17ed238c..923e6ecc0a 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -472,6 +472,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix) /* If we already have a sparse-checkout file, use it. */ if (res >= 0) { free(sparse_filename); + clear_pattern_list(&pl); return update_working_directory(NULL); } From patchwork Tue Jun 4 10:13:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685000 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 8FBAD1448F3 for ; Tue, 4 Jun 2024 10:13:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496003; cv=none; b=BwdC7hzZ/YBJGURyXjP0Y1Ei00LF0py/xvInfewbTVaXvnXCZON+AeCOqeh4z05fTWBIZuCXT6qD+Ah1L8/2jeEEHNgBjriqjV2fnfvcwXr+pC09zR5g5blzcVl7TTBaUEqv71Q3DFWePJqrMdEnq2j+tSAMRpDJJ4WDAJyeOn8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496003; c=relaxed/simple; bh=pclLH5xWChQEwgL3WovA8aok6XFXqtdZbb60wGEJwlI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Ve3Iv2KPza6P29v8IzR3E6BUVMRSSZofwYmawIkN+BNr0owIToabTLor3uK6MLellSA44tFWooqUReLIPQWw3ah1pvV0cXg69FPJczcq5CbqtA4kWF1v+qPA7CSagaTxmwhvHSzuHOoJMmI/3WrzECatEhtYq0DHi6vb3sSsBQc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21494 invoked by uid 109); 4 Jun 2024 10:13:21 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:21 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18367 invoked by uid 111); 4 Jun 2024 10:13:18 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:18 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:20 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 05/13] dir.c: free removed sparse-pattern hashmap entries Message-ID: <20240604101320.GE1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In add_pattern_to_hashsets(), we remove entries from the recursive_hashmap when adding similar ones to the parent_hashmap. I won't pretend to understand all of what's going on here, but there's an obvious leak: whatever we removed from recursive_hashmap is not referenced anywhere else, and is never free()d. We can easily fix this by asking the hashmap to return a pointer to the old entry. This makes t7002 now completely leak-free. Signed-off-by: Jeff King --- dir.c | 8 +++++++- t/t7002-mv-sparse-checkout.sh | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index 9cc4a68fbc..d812d521b0 100644 --- a/dir.c +++ b/dir.c @@ -810,6 +810,8 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern if (given->patternlen > 2 && !strcmp(given->pattern + given->patternlen - 2, "/*")) { + struct pattern_entry *old; + if (!(given->flags & PATTERN_FLAG_NEGATIVE)) { /* Not a cone pattern. */ warning(_("unrecognized pattern: '%s'"), given->pattern); @@ -835,7 +837,11 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern } hashmap_add(&pl->parent_hashmap, &translated->ent); - hashmap_remove(&pl->recursive_hashmap, &translated->ent, &data); + old = hashmap_remove_entry(&pl->recursive_hashmap, translated, ent, &data); + if (old) { + free(old->pattern); + free(old); + } free(data); return; } diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh index 26582ae4e5..57969ce805 100755 --- a/t/t7002-mv-sparse-checkout.sh +++ b/t/t7002-mv-sparse-checkout.sh @@ -2,6 +2,7 @@ test_description='git mv in sparse working trees' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh setup_sparse_checkout () { From patchwork Tue Jun 4 10:13:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685001 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 47AEC1448EE for ; Tue, 4 Jun 2024 10:13:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496006; cv=none; b=AofAcu992miMxPK2NdzxmEzYJktOZFdxDCAZQF3y+pqlFWtbDNZbDZWH4qdmotIw9JvOGSwFrEAY0H71SVPKY7YG245jL1qM52prXtwrVkopOu3okivv7KBQNYZ4m75uas5Ad19zixSZ23D5xyEFIB9ylSO0DxICzxMZGmt0w/k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496006; c=relaxed/simple; bh=XzsvB7+nBcewK4znrLrew30Ul11NbmGOQBFYrvI/d9c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pg95YeA/BsUopNJRiB/6xDn7Kwo8SCz/iwFcxBv6632AlrDZahdNAOnZdE0qznbySFH6m9nzoCi74b1DKkcKK9M8GllkcxZTq1r36tk8F8TRjboyR6YyV073lsNjPQcKaCMTwMgZCOP8nxPxWd6Un0icdrNXPF8dL7eVsXlUOsQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21502 invoked by uid 109); 4 Jun 2024 10:13:24 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:24 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18372 invoked by uid 111); 4 Jun 2024 10:13:21 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:21 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:22 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 06/13] dir.c: always copy input to add_pattern() Message-ID: <20240604101322.GF1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> The add_pattern() function has a subtle and undocumented gotcha: the pattern string you pass in must remain valid as long as the pattern_list is in use (and nor do we take ownership of it). This is easy to get wrong, causing either subtle bugs (because you free or reuse the string buffer) or leaks (because you copy the string, but don't track ownership separately). All of this "pattern" code was originally the "exclude" mechanism. So this _usually_ works OK because you add entries in one of two ways: 1. From the command-line (e.g., "--exclude"), in which case we're pointing to an argv entry which remains valid for the lifetime of the program. 2. From a file (e.g., ".gitignore"), in which case we read the whole file into a buffer, attach it to the pattern_list's "filebuf" entry, then parse the buffer in-place (adding NULs). The strings point into the filebuf, which is cleaned up when the whole pattern_list goes away. But other code, like sparse-checkout, reads individual lines from stdin and passes them one by one to add_pattern(), leaking each. We could fix this by refactoring it to take in the whole buffer at once, like (2) above, and stuff it in "filebuf". But given how subtle the interface is, let's just fix it to always copy the string. That seems at first like we'd be wasting extra memory, but we can mitigate that: a. The path_pattern struct already uses a FLEXPTR, since we sometimes make a copy (when we see "foo/", we strip off the trailing slash, requiring a modifiable copy of the string). Since we'll now always embed the string inside the struct, we can switch to the regular FLEX_ARRAY pattern, saving us 8 bytes of pointer. So patterns with a trailing slash and ones under 8 bytes actually get smaller. b. Now that we don't need the original string to hang around, we can get rid of the "filebuf" mechanism entirely, and just free the file contents after parsing. Since files are the sources we'd expect to have the largest pattern sets, we should mostly break even on stuffing the same data into the individual structs. This patch just adjusts the add_pattern() interface; it doesn't fix any leaky callers yet. Signed-off-by: Jeff King --- dir.c | 15 +++++---------- dir.h | 3 ++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/dir.c b/dir.c index d812d521b0..8308d167c8 100644 --- a/dir.c +++ b/dir.c @@ -925,12 +925,7 @@ void add_pattern(const char *string, const char *base, int nowildcardlen; parse_path_pattern(&string, &patternlen, &flags, &nowildcardlen); - if (flags & PATTERN_FLAG_MUSTBEDIR) { - FLEXPTR_ALLOC_MEM(pattern, pattern, string, patternlen); - } else { - pattern = xmalloc(sizeof(*pattern)); - pattern->pattern = string; - } + FLEX_ALLOC_MEM(pattern, pattern, string, patternlen); pattern->patternlen = patternlen; pattern->nowildcardlen = nowildcardlen; pattern->base = base; @@ -972,7 +967,6 @@ void clear_pattern_list(struct pattern_list *pl) for (i = 0; i < pl->nr; i++) free(pl->patterns[i]); free(pl->patterns); - free(pl->filebuf); clear_pattern_entry_hashmap(&pl->recursive_hashmap); clear_pattern_entry_hashmap(&pl->parent_hashmap); @@ -1166,23 +1160,23 @@ static int add_patterns(const char *fname, const char *base, int baselen, } add_patterns_from_buffer(buf, size, base, baselen, pl); + free(buf); return 0; } static int add_patterns_from_buffer(char *buf, size_t size, const char *base, int baselen, struct pattern_list *pl) { + char *orig = buf; int i, lineno = 1; char *entry; hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0); hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0); - pl->filebuf = buf; - if (skip_utf8_bom(&buf, size)) - size -= buf - pl->filebuf; + size -= buf - orig; entry = buf; @@ -1222,6 +1216,7 @@ int add_patterns_from_blob_to_list( return r; add_patterns_from_buffer(buf, size, base, baselen, pl); + free(buf); return 0; } diff --git a/dir.h b/dir.h index b9e8e96128..c8ff308fae 100644 --- a/dir.h +++ b/dir.h @@ -62,7 +62,6 @@ struct path_pattern { */ struct pattern_list *pl; - const char *pattern; int patternlen; int nowildcardlen; const char *base; @@ -74,6 +73,8 @@ struct path_pattern { * and from -1 decrementing for patterns from CLI args. */ int srcpos; + + char pattern[FLEX_ARRAY]; }; /* used for hashmaps for cone patterns */ From patchwork Tue Jun 4 10:13:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685002 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 DD7C214532F for ; Tue, 4 Jun 2024 10:13:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496008; cv=none; b=T9OEbbgvuEOiZeWtgD/v4kcicFRC4iDJEGOSAUFyRf5u8lPaLVhaO5ZKQWdafXt5Uv4UU/vSse5yenpGvQZC/ZeQs5OH+/r+Uxt5wgxK4uUH9PD4jxVtArbOiQo0VdvJVhqaVm8fs80RJQ3Eq2yG7ovJHZgFEQiEHm6Hpkf4llw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496008; c=relaxed/simple; bh=S1m6CMFs1uu28irQFctu2UeHjXha6ebGGRgGnE0bgMQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MXHN/tcQ4uGmIe9AlCG6vLtpAT77qpnMtsvrlAA+2CFDAQKeVtji3cKyefYsd5iG/auxWR6bnE8cIr2bW2YFKvJxu96jTyrq2JfVeY4l6mVCp/EiSLGvBxXb+m5Sj8Ob9IH8xhgZvHxu4zZjzxLtybdXIWF/aIyG4x0+EaUfLJI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21512 invoked by uid 109); 4 Jun 2024 10:13:26 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:26 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18377 invoked by uid 111); 4 Jun 2024 10:13:23 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:23 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:25 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 07/13] sparse-checkout: reuse --stdin buffer when reading patterns Message-ID: <20240604101325.GG1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> When we read patterns from --stdin, we loop on strbuf_getline(), and detach each line we read to pass into add_pattern(). This used to be necessary because add_pattern() required that the pattern strings remain valid while the pattern_list was in use. But it also created a leak, since we didn't record the detached buffers anywhere else. Now that add_pattern() has been modified to make its own copy of the strings, we can stop detaching and fix the leak. This fixes 4 leaks detected in t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 923e6ecc0a..75c07d5bb4 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -585,11 +585,10 @@ static void add_patterns_from_input(struct pattern_list *pl, if (file) { struct strbuf line = STRBUF_INIT; - while (!strbuf_getline(&line, file)) { - size_t len; - char *buf = strbuf_detach(&line, &len); - add_pattern(buf, empty_base, 0, pl, 0); - } + while (!strbuf_getline(&line, file)) + add_pattern(line.buf, empty_base, 0, pl, 0); + + strbuf_release(&line); } else { for (i = 0; i < argc; i++) add_pattern(argv[i], empty_base, 0, pl, 0); From patchwork Tue Jun 4 10:13:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685003 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 1741714535B for ; Tue, 4 Jun 2024 10:13:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496010; cv=none; b=LQtnJleHvRwyxr+9swXj72zQWUiuO3tvsYtGXwKx6d/j4nCTahXuSV5BnNjmD/mi/CUhf2pU9k3yl62T8VaKUPBf1wJouFQA3WGYDXIJZbp5fehQ9EAdQ9cESdnzVSoWU5MtH5BYAnfuh2NL+FcgfBc8MmUZOfJrFwwQmTvVPx8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496010; c=relaxed/simple; bh=ASJX00FrRKyzwxf8aeQSZd6qCVM9EYKX1CgI5CadC+8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ReihEHYlV+3WBfT8dxo+rGOwz7qM70xIEln0HTClJYTSrs+ftt5NHdmZNe/9Ag99+JhQamr96llesBCEtuRL2yBdBrpyihgSbq5MaFMGka0/5to614usPD8Kg75aawH7PZX+1lbnuLUAA7VAux7N9/dHzMCXt1UT2HOWeIjAQbE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21522 invoked by uid 109); 4 Jun 2024 10:13:29 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:29 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18382 invoked by uid 111); 4 Jun 2024 10:13:25 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:25 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:27 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 08/13] sparse-checkout: always free "line" strbuf after reading input Message-ID: <20240604101327.GH1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In add_patterns_from_input(), we may read lines from a file with a loop like this: while (!strbuf_getline(&line, file)) { ... strbuf_to_cone_pattern(&line, pl); } /* we don't strbuf_release(&line) here! */ This generally is OK because strbuf_to_cone_pattern() consumes the buffer via strbuf_detach(). But we can leak in a few cases: 1. We don't always consume the buffer! If the line ends up empty after trimming, we leave strbuf_to_cone_pattern() without detaching. In most cases this is OK, because a subsequent getline() call will use the same buffer. But if you had an empty line at the end of file, for example, it would leak. 2. Even if strbuf_to_cone_pattern() always consumed the buffer, there's a subtle issue with strbuf_getline(). As we saw in 94e2aa555e (strbuf: fix leak when `appendwholeline()` fails with EOF, 2024-05-27), it's possible for it to return EOF with an allocated buffer (e.g., if the underlying getdelim() call saw an error). So we should always strbuf_release() after finishing a read loop like this. Note that even the code to read patterns from argv has the same problem. Because that also uses strbuf_to_cone_pattern(), we stuff each argv entry into a strbuf. It uses the same "line" strbuf as the getline code, but we should position the strbuf_release() to cover both code paths. This fixes at least 9 leaks found in t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 75c07d5bb4..8f8f5c359f 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -581,6 +581,7 @@ static void add_patterns_from_input(struct pattern_list *pl, strbuf_to_cone_pattern(&line, pl); } } + strbuf_release(&line); } else { if (file) { struct strbuf line = STRBUF_INIT; From patchwork Tue Jun 4 10:13:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685004 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 D1AAE144D10 for ; Tue, 4 Jun 2024 10:13:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496013; cv=none; b=oTTqT0oulXe4kPlXvgteTVRfXuRnhip0Pk/JbGxtlz224aT311adbKg0xQMg5Gg6WQFvoOGpaTd+Nffhsc1HsI8zUOb32OS52qiGICC5JY/o43eB8x99pATPGSxYYPIUlDV0mk+AbsF29jwcL5BdNJyVYYNGcuU7uiCK6M6QiUk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496013; c=relaxed/simple; bh=gvCK55F0sDRjL2Ty3tjrGCVkN6s9axj+fGWc7JHQ6Qk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=RjxUoQGRkU/OzD3xnhF0w0f3hwnfKkye8NjWH58qy3rwGm34UVCZETnCn1ZBfCNW0M3KnGd1FvpaGcevVHVli9tiSMRKiSii0whvDYOT8pJQhdPv0sWpAChEJx2dQ57QCzS9EAo92zvv20UqRY96BZwk6z2hnhGErE7N4qdn2fU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21529 invoked by uid 109); 4 Jun 2024 10:13:31 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:31 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18387 invoked by uid 111); 4 Jun 2024 10:13:28 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:28 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:30 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 09/13] sparse-checkout: refactor temporary sparse_checkout_patterns Message-ID: <20240604101330.GI1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In update_working_directory(), we take in a pattern_list, attach it to the repository index by assigning it to index->sparse_checkout_patterns, and then call unpack_trees. Afterwards, we remove it by setting index->sparse_checkout_patterns back to NULL. But there are two possible leaks here: 1. If the index already had a populated sparse_checkout_patterns, we've obliterated it. We can fix this by saving and restoring it, rather than always setting it back to NULL. 2. We may call the function with a NULL pattern_list, expecting it to use the on-disk sparse file. In that case, the index routines will lazy-load the sparse patterns automatically. But now at the end of the function when we restore the patterns, we'll leak those lazy-loaded ones! We can fix this by freeing the pattern list before overwriting its pointer whenever it does not match what was passed in (in practice this should only happen when the passed-in list is NULL, but this is erring on the defensive side). Together these remove 48 indirect leaks found in t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 8f8f5c359f..b84d2e1c80 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -205,11 +205,13 @@ static int update_working_directory(struct pattern_list *pl) struct unpack_trees_options o; struct lock_file lock_file = LOCK_INIT; struct repository *r = the_repository; + struct pattern_list *old_pl; /* If no branch has been checked out, there are no updates to make. */ if (is_index_unborn(r->index)) return UPDATE_SPARSITY_SUCCESS; + old_pl = r->index->sparse_checkout_patterns; r->index->sparse_checkout_patterns = pl; memset(&o, 0, sizeof(o)); @@ -241,7 +243,12 @@ static int update_working_directory(struct pattern_list *pl) clean_tracked_sparse_directories(r); - r->index->sparse_checkout_patterns = NULL; + if (r->index->sparse_checkout_patterns != pl) { + clear_pattern_list(r->index->sparse_checkout_patterns); + FREE_AND_NULL(r->index->sparse_checkout_patterns); + } + r->index->sparse_checkout_patterns = old_pl; + return result; } From patchwork Tue Jun 4 10:13:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685005 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 775EB144D01 for ; Tue, 4 Jun 2024 10:13:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496015; cv=none; b=CXqIJahqKZtwBeM10MAwTmOUXJ4VeCUQHw7oPCht9h2ce1fq7cmHnL4rH9s3fNfApKcx7/i4ppBtsdqEzTJ+uSSqdg5HyXp12yVEQPLiaP/0W1AS1NIHgCUKlT042WJE5f7fLbmSx+VgxZB4wyWR6yUdp+MbeRuq5A6D+pFv3gk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496015; c=relaxed/simple; bh=T7TbIYQBTc5u7imbbj+7v05rLjlFHADTRid6arMA9t4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tdTuoYA18C2AtsqYJSQ/XU9SfXNhRb+WdmBQTNB9H8CSQ2XyfKcd/lYQ11rSD0dPTsu+bd/Q51BOGwoHhLXVAGX16zg3nqlZbQqWHYehWQQ1NgEzMZBwgT2EDKDvs92zIWzVucq+0b4Ko+jW+9Ci25bhUW/Ez/E7KqBjdZm5Fdo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21543 invoked by uid 109); 4 Jun 2024 10:13:34 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:34 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18392 invoked by uid 111); 4 Jun 2024 10:13:30 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:30 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:32 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 10/13] sparse-checkout: free sparse_filename after use Message-ID: <20240604101332.GJ1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> We allocate a heap buffer via get_sparse_checkout_filename(). Most calls remember to free it, but sparse_checkout_init() forgets to, causing a leak. Ironically, it remembers to do so in the error return paths, but not in the path that makes it all the way to the function end! Fixing this clears up 6 leaks from t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index b84d2e1c80..79342480eb 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -500,6 +500,8 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix) return 0; } + free(sparse_filename); + add_pattern("/*", empty_base, 0, &pl, 0); add_pattern("!/*/", empty_base, 0, &pl, 0); pl.use_cone_patterns = init_opts.cone_mode; From patchwork Tue Jun 4 10:13:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685006 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 9DD88145A02 for ; Tue, 4 Jun 2024 10:13:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496018; cv=none; b=lhOLYpaKgDO8qb3vhl1f4cpi33i9biRALnwteKQ7gTqvbNKbH/wSHM643i5bs/3SwVMw3SBCrsimAr4lW1w01imNgwaFx/5boFgtdSTSmt90/TNFSyM8SdPStnQSm/L44YB8D8P7X0Pw8fb4zj4STZkfRpmlpUuhVtwVVkmisvk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496018; c=relaxed/simple; bh=WFEpOu+PtOu8ON0eS3y9A79Z+jbL2Gz7cGGQrhU2PQE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OkrBhD3obV/YT0XWQh6HWqn5o9gyLeEfp9myBmOiGC8yBmlTecblRwXw3uSOboHPE8OqRUo9Vdbpuln8YjM/naQSqN4Urq3C7AkAhakziNakABNR+VD9ModvGxhNGN6qzvjeoAqTn02m5Hc3YEmBgNqSvbrpQS/0ROGeFaxGJ94= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21552 invoked by uid 109); 4 Jun 2024 10:13:36 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:36 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18397 invoked by uid 111); 4 Jun 2024 10:13:33 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:33 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:35 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 11/13] sparse-checkout: free pattern list in sparse_checkout_list() Message-ID: <20240604101335.GK1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In sparse_checkout_list(), we create a pattern_list that needs to eventually be cleared. We remember to do so in the regular code path, but the cone-mode path does an early return, and forgets to clean up. We could fix the leak by adding a new call to clear_pattern_list(). But we can simplify even further by just skipping the early return, pushing the other code path (which consists now of only one line!) into an else block. That also matches the same cone/non-cone if/else used in some other functions. This fixes 15 leaks found in t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 79342480eb..fb43bb7577 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -95,11 +95,10 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix) quote_c_style(sl.items[i].string, NULL, stdout, 0); printf("\n"); } - - return 0; + } else { + write_patterns_to_file(stdout, &pl); } - write_patterns_to_file(stdout, &pl); clear_pattern_list(&pl); return 0; From patchwork Tue Jun 4 10:13:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685007 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 209A0144D2D for ; Tue, 4 Jun 2024 10:13:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496020; cv=none; b=flDkmYtmc476/AtmDPBZD1gKd+awpwkS7VKWrL+MyFfizP0TQNZyBJotOrBu6Sm1z81JeAeOae4nZSvOYwf2gOaMQ/X8p3TjPMKNQF5Xg9u7Crwd0npMMeiJc4nrT1CPvPzXzOEAvJx3l9N1i+x0ykJg8wXso4jwi63bOvjmsLA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496020; c=relaxed/simple; bh=87YmY9b0D9MyjTaWynAiIN6LPEUO0ZlDySTQmG0jycs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tQl0PZQnY7lMoMdnJCmfK2femsjCRcrUR40ugPinVr0F8T5GhX8x4gshFTB5+NBbm8MW5g5JOthzZezkJSEsUgEhtrW7t9NqLY+MWgZkltUL1N/glKvWPcTnV9D9w5sClMLs1vGCoKVsaZ+DHDKBgswjy9Y8N/VTqJs40yaBv+M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21561 invoked by uid 109); 4 Jun 2024 10:13:39 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:39 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18402 invoked by uid 111); 4 Jun 2024 10:13:35 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:35 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:37 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 12/13] sparse-checkout: free string list after displaying Message-ID: <20240604101337.GL1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In sparse_checkout_list(), we put the hashmap entries into a string_list so we can sort them. But after printing, we forget to free the list. This patch drops 5 leaks from t1091. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index fb43bb7577..e648e035ab 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -95,6 +95,8 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix) quote_c_style(sl.items[i].string, NULL, stdout, 0); printf("\n"); } + + string_list_clear(&sl, 0); } else { write_patterns_to_file(stdout, &pl); } From patchwork Tue Jun 4 10:13:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13685008 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 A8A7E144D2D for ; Tue, 4 Jun 2024 10:13:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496023; cv=none; b=oVdmfuSlA2sCltNXZPyWwCWaV1UZk6S1xFmqSBA9woNaCOfib9tBD+PFVt60pN+RhIH1SHKyNy07P5xL3hw4ot8i1D13Ok92MHLLBKOFJbvUCDSTYDuxl4TQc++QKGYXBrNgSYkfp21KUb/s5UI+QH9EijmDqf6yXwhxgucHFX4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717496023; c=relaxed/simple; bh=8egrULMIwM+ub4Unk5iAaNgQUWCrwr1gofvZnj0MLjM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=njKOee7L0eudg/2vDgUgWoU9CGY/l53P2UaKIp+iVsYbwZKvrKUX5E4C191Q1XJRq4fQBhNQGnXLPxCiBQGglIpu+U44KSoBMhuDCSuFYNxhXYsd8Dryau73sN/kTdkYPTIASj7WOsacre5jZmep9Bqim2GADQ3mzPGbeDWZGOQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 21571 invoked by uid 109); 4 Jun 2024 10:13:41 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 04 Jun 2024 10:13:41 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 18407 invoked by uid 111); 4 Jun 2024 10:13:38 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 04 Jun 2024 06:13:38 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 4 Jun 2024 06:13:40 -0400 From: Jeff King To: git@vger.kernel.org Cc: Junio C Hamano , Patrick Steinhardt Subject: [PATCH v2 13/13] sparse-checkout: free duplicate hashmap entries Message-ID: <20240604101340.GM1304593@coredump.intra.peff.net> References: <20240604100814.GA1304520@coredump.intra.peff.net> 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: <20240604100814.GA1304520@coredump.intra.peff.net> In insert_recursive_pattern(), we create a new pattern_entry to insert into the parent_hashmap. If we find that the same entry already exists in the hashmap, we skip adding the new one. But we forget to free the new one, creating a leak. We can fix it by cleaning up the discarded entry. It would probably be possible to avoid creating it in the first place, but it's non-trivial. We'd have to define a "keydata" struct that lets us compare the existing entries to the broken-out fields. It's probably not worth the complexity, so we'll punt on that for now. There is one subtlety here: our insertion is happening in a loop, with each iteration looking at the pattern we just inserted (hence the "recursive" in the name). So if we skip insertion, what do we look at? The obvious answer is that we should remember the existing duplicate we found and use that. But I _think_ in that case, we probably already have all of the recursive bits already (from when the original entry was added). And so just breaking out of the loop would be correct. But I'm not 100% sure on that; after all, the original leaky code could have done the same break, but it didn't. So I went with the "obvious answer" above, which has no chance of changing the behavior aside from fixing the leak. With this patch, t1091 can now be marked leak-free. Signed-off-by: Jeff King --- builtin/sparse-checkout.c | 9 ++++++++- t/t1091-sparse-checkout-builtin.sh | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index e648e035ab..3f2bfce8fa 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -523,6 +523,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat char *slash = strrchr(e->pattern, '/'); char *oldpattern = e->pattern; size_t newlen; + struct pattern_entry *dup; if (!slash || slash == e->pattern) break; @@ -533,8 +534,14 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat e->pattern = xstrndup(oldpattern, newlen); hashmap_entry_init(&e->ent, fspathhash(e->pattern)); - if (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL)) + dup = hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL); + if (!dup) { hashmap_add(&pl->parent_hashmap, &e->ent); + } else { + free(e->pattern); + free(e); + e = dup; + } } } diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index ab3a105fff..8c5cd651b4 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -8,6 +8,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME GIT_TEST_SPLIT_INDEX=false export GIT_TEST_SPLIT_INDEX +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh list_files() {