From patchwork Fri Jul 2 09:57:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12355781 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E47EFC11F68 for ; Fri, 2 Jul 2021 09:57:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C21D3613BC for ; Fri, 2 Jul 2021 09:57:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231318AbhGBKAK (ORCPT ); Fri, 2 Jul 2021 06:00:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37208 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230296AbhGBKAK (ORCPT ); Fri, 2 Jul 2021 06:00:10 -0400 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 39436C061764 for ; Fri, 2 Jul 2021 02:57:38 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id q18-20020a1ce9120000b02901f259f3a250so5968785wmc.2 for ; Fri, 02 Jul 2021 02:57:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Jll18OVzZewk75+GFM5FnqX7JXR7rlReOsSpV1sOg+U=; b=Z4cYMhWijzFIXVMT5qouFXTwYiU1TXvyaY459jxbzznlX58MIbAu0ve53gzStRvsK1 v08dQJw2GekJZzmfFB+4C6nm0jtLTYjkXdJIoRbfU+VIxOqoZlF8eTLQ70t6DipqTdIJ VQJ6JLVONIkUB1p+90vXCDYsC5H04FqXAoz36HwAtqiFJfLQ//0F25G3atpsVxI9pqVA woKjM3xjqBrxrkC2iAgrPN0XwtWx77J0GqyPP4/4L1ElOfbXpH5ZcXv871ENMWLUXMZ4 Fy48+CEB10KNr3VDYEnuh90LA1EMaUR1qwOhLw8pGnEKMEnpFn5Nr5ZVaH8MQlg8mpfw lRpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Jll18OVzZewk75+GFM5FnqX7JXR7rlReOsSpV1sOg+U=; b=eu3BUUcfT/qnECA5HWEmXs80rjzhWnrDrAT8pQOT9ba3kCWY5ON/UnVNOcCXVUjW4f RatFj11XaZxivqqmTmign/Ic8Thd7Rcx46Pv3x182qadLg22vc7oQM0RrRpziSE0woGm jomBsgNOSw0RUi/9ekzDLgk6xH/XOISIDwuDb5j8llLtqFo5UFgQkZk2kxjAhBbOUjoP uzy20VW/MoN5OpzO25oSJugY0ngwH3y+8VfENKln4/FteLG/2VHNWngmwML/jXkScMmc wLd7hbL8w9ypDhBUqWtWW6iSSrxwk/NguXjBhCfTEXsBN0MCwLJBVVxI5MYNWyUL1Png zYiQ== X-Gm-Message-State: AOAM532kaBJ+jcDxE3QNwsuOwBDfdjYhn7PJUKvlmuKDwZcf/ohlQkFm C4N7uk4x882mi0B2Mr6OEoS8r5tDnaldKA== X-Google-Smtp-Source: ABdhPJxHyk6J3PPxmV56VjqHStzUfEYbsjTAZdi+Vc4R5wnUtvDob8UkHDr8/zUXxNm0E2nG2tRHoQ== X-Received: by 2002:a7b:c39a:: with SMTP id s26mr4594415wmj.115.1625219856646; Fri, 02 Jul 2021 02:57:36 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id m6sm3307713wrw.9.2021.07.02.02.57.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Jul 2021 02:57:36 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , Andrei Rybak , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFy?= =?utf-8?b?bWFzb24=?= Subject: [PATCH v4 1/3] bundle cmd: stop leaking memory from parse_options_cmd_bundle() Date: Fri, 2 Jul 2021 11:57:30 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.632.g49a94b9226d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix a memory leak from the prefix_filename() function introduced with its use in 3b754eedd5 (bundle: use prefix_filename with bundle path, 2017-03-20). As noted in that commit the leak was intentional as a part of being sloppy about freeing resources just before we exit, I'm changing this because I'll be fixing other memory leaks in the bundle API (including the library version) in subsequent commits. It's easier to reason about those fixes if valgrind runs cleanly at the end without any leaks whatsoever. An earlier version of this change[1] went out of its way to not leak memory on the die() codepaths here, but doing so will only avoid reports of potential leaks under heap-only leak trackers such as valgrind, not the SANITIZE=leak mode. Avoiding those leaks as well might be useful to enable us to run cleanly under the likes of valgrind in the future. But for now the relative verbosity of the resulting code, and the fact that we don't have some valgrind or SANITIZE=leak mode as part of our CI (it's only run ad-hoc, see [2]), means we're not worrying about that for now. 1. https://lore.kernel.org/git/87v95vdxrc.fsf@evledraar.gmail.com/ 2. https://lore.kernel.org/git/87czsv2idy.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/bundle.c | 62 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index ea6948110b0..15e2bd61965 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -46,7 +46,7 @@ static int parse_options_cmd_bundle(int argc, const char* prefix, const char * const usagestr[], const struct option options[], - const char **bundle_file) { + char **bundle_file) { int newargc; newargc = parse_options(argc, argv, NULL, options, usagestr, PARSE_OPT_STOP_AT_NON_OPTION); @@ -61,7 +61,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { int progress = isatty(STDERR_FILENO); struct strvec pack_opts; int version = -1; - + int ret; struct option options[] = { OPT_SET_INT('q', "quiet", &progress, N_("do not show progress meter"), 0), @@ -76,7 +76,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { N_("specify bundle format version")), OPT_END() }; - const char* bundle_file; + char *bundle_file; argc = parse_options_cmd_bundle(argc, argv, prefix, builtin_bundle_create_usage, options, &bundle_file); @@ -94,75 +94,95 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { if (!startup_info->have_repository) die(_("Need a repository to create a bundle.")); - return !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version); + ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version); + free(bundle_file); + return ret; } static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { struct bundle_header header; int bundle_fd = -1; int quiet = 0; - + int ret; struct option options[] = { OPT_BOOL('q', "quiet", &quiet, N_("do not show bundle details")), OPT_END() }; - const char* bundle_file; + char *bundle_file; argc = parse_options_cmd_bundle(argc, argv, prefix, builtin_bundle_verify_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ memset(&header, 0, sizeof(header)); - if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) - return 1; + if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { + ret = 1; + goto cleanup; + } close(bundle_fd); - if (verify_bundle(the_repository, &header, !quiet)) - return 1; + if (verify_bundle(the_repository, &header, !quiet)) { + ret = 1; + goto cleanup; + } + fprintf(stderr, _("%s is okay\n"), bundle_file); - return 0; + ret = 0; +cleanup: + free(bundle_file); + return ret; } static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) { struct bundle_header header; int bundle_fd = -1; - + int ret; struct option options[] = { OPT_END() }; - const char* bundle_file; + char *bundle_file; argc = parse_options_cmd_bundle(argc, argv, prefix, builtin_bundle_list_heads_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ memset(&header, 0, sizeof(header)); - if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) - return 1; + if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { + ret = 1; + goto cleanup; + } close(bundle_fd); - return !!list_bundle_refs(&header, argc, argv); + ret = !!list_bundle_refs(&header, argc, argv); +cleanup: + free(bundle_file); + return ret; } static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) { struct bundle_header header; int bundle_fd = -1; - + int ret; struct option options[] = { OPT_END() }; - const char* bundle_file; + char *bundle_file; argc = parse_options_cmd_bundle(argc, argv, prefix, builtin_bundle_unbundle_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ memset(&header, 0, sizeof(header)); - if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) - return 1; + if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { + ret = 1; + goto cleanup; + } if (!startup_info->have_repository) die(_("Need a repository to unbundle.")); - return !!unbundle(the_repository, &header, bundle_fd, 0) || + ret = !!unbundle(the_repository, &header, bundle_fd, 0) || list_bundle_refs(&header, argc, argv); +cleanup: + free(bundle_file); + return ret; } int cmd_bundle(int argc, const char **argv, const char *prefix) From patchwork Fri Jul 2 09:57:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12355785 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69C94C11F6B for ; Fri, 2 Jul 2021 09:57:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5189C61411 for ; Fri, 2 Jul 2021 09:57:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231372AbhGBKAM (ORCPT ); Fri, 2 Jul 2021 06:00:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231252AbhGBKAL (ORCPT ); Fri, 2 Jul 2021 06:00:11 -0400 Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DDBA6C061762 for ; Fri, 2 Jul 2021 02:57:38 -0700 (PDT) Received: by mail-wr1-x42a.google.com with SMTP id l8so11700666wry.13 for ; Fri, 02 Jul 2021 02:57:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=yjQU55FJ5FWAlV+EeUxRuV3c3kNRAyOfNe2csuWxUjY=; b=JPdleGIovWqIiVEo7I252Jy+1b5aANX4x2sXL0tHyu9xnedaqnj8E190ZGo+vxdkjQ zX1CZ3udLRU/p61K3aRbzEZ+tNtfVghvaKUCKXoKn0PcXfXGjEC9j8SgqxzQE7srucr0 HqU0FoQ/ast0CMPBc1KFjzUOAkQNvzutp4zuZZpR9V6WgEMF2/sPHK32bepuoR3cv853 kf08DCrR3L9222eyS6tMxE/kiP4+uHGm1WvSusNvMaId2sn5xF0busYiqf1vHNB3mYkb 0oRPywVwKDAPD6BOpLRvSQbXjF+VnvCi7iHPOccG/yVU3HInMgjMH5pYooXFyrQ8LpEr P89Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=yjQU55FJ5FWAlV+EeUxRuV3c3kNRAyOfNe2csuWxUjY=; b=t3ClqDImRUVgsBAyw/7pID3jGbn4FSE0W0eBW0cEI4eFczjTJFskXqsZ1VvBoZ4Qq1 Njo/1R5hv4ijLD19XwvJVADHdjL9CHkgPcdkkKRT0dpTncyL5YCiIlsQWDWIJrpPGpe7 Z7CkSAgnwRoEQNW9Y0QOD1chNr8VLyRmhxARPOopKloUGajDX5k476eEhAJfNMUK3dVN Oz8lHbGylcTD0ZSkpP4Mw6Gm613mhSSZ3Mb2E3sIic3E59HV0BXDitz4gSDQPNUTnYuv XQFmsskalPMoTYEl9tU+kv1KPF4n21Hw05eBg1Di6JL9WX4yegOxXlegkL8yELphYelW O59Q== X-Gm-Message-State: AOAM532ugmoQ5KiTRLqRdNuthBolHnCaEGRqN1EgLqjgcj1swR4wa88g j6thM5AEkMLhzx40SCL20IAtra+iJJiX3w== X-Google-Smtp-Source: ABdhPJxMOuYUHD7nfgo5tro8ORw++mJb0tEHaU7BBMu5J/Yuev8ukSy5QMV3778gn4rxkIxBs0u7Qg== X-Received: by 2002:adf:f048:: with SMTP id t8mr4784758wro.35.1625219857315; Fri, 02 Jul 2021 02:57:37 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id m6sm3307713wrw.9.2021.07.02.02.57.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Jul 2021 02:57:36 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , Andrei Rybak , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFy?= =?utf-8?b?bWFzb24=?= Subject: [PATCH v4 2/3] bundle.c: use a temporary variable for OIDs and names Date: Fri, 2 Jul 2021 11:57:31 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.632.g49a94b9226d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In preparation for moving away from accessing the OID and name via the "oid" and "name" slots in a subsequent commit, change the code that accesses it to use named variables. This makes the subsequent change smaller. Signed-off-by: Ævar Arnfjörð Bjarmason --- bundle.c | 26 ++++++++++++++++++-------- transport.c | 6 ++++-- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/bundle.c b/bundle.c index 693d6195514..7210e5e7105 100644 --- a/bundle.c +++ b/bundle.c @@ -156,6 +156,9 @@ static int list_refs(struct ref_list *r, int argc, const char **argv) int i; for (i = 0; i < r->nr; i++) { + struct object_id *oid; + const char *name; + if (argc > 1) { int j; for (j = 1; j < argc; j++) @@ -164,8 +167,10 @@ static int list_refs(struct ref_list *r, int argc, const char **argv) if (j == argc) continue; } - printf("%s %s\n", oid_to_hex(&r->list[i].oid), - r->list[i].name); + + oid = &r->list[i].oid; + name = r->list[i].name; + printf("%s %s\n", oid_to_hex(oid), name); } return 0; } @@ -194,15 +199,17 @@ int verify_bundle(struct repository *r, repo_init_revisions(r, &revs, NULL); for (i = 0; i < p->nr; i++) { struct ref_list_entry *e = p->list + i; - struct object *o = parse_object(r, &e->oid); + const char *name = e->name; + struct object_id *oid = &e->oid; + struct object *o = parse_object(r, oid); if (o) { o->flags |= PREREQ_MARK; - add_pending_object(&revs, o, e->name); + add_pending_object(&revs, o, name); continue; } if (++ret == 1) error("%s", message); - error("%s %s", oid_to_hex(&e->oid), e->name); + error("%s %s", oid_to_hex(oid), name); } if (revs.pending.nr != p->nr) return ret; @@ -219,19 +226,22 @@ int verify_bundle(struct repository *r, for (i = 0; i < p->nr; i++) { struct ref_list_entry *e = p->list + i; - struct object *o = parse_object(r, &e->oid); + const char *name = e->name; + struct object_id *oid = &e->oid; + struct object *o = parse_object(r, oid); assert(o); /* otherwise we'd have returned early */ if (o->flags & SHOWN) continue; if (++ret == 1) error("%s", message); - error("%s %s", oid_to_hex(&e->oid), e->name); + error("%s %s", oid_to_hex(oid), name); } /* Clean up objects used, as they will be reused. */ for (i = 0; i < p->nr; i++) { struct ref_list_entry *e = p->list + i; - commit = lookup_commit_reference_gently(r, &e->oid, 1); + struct object_id *oid = &e->oid; + commit = lookup_commit_reference_gently(r, oid, 1); if (commit) clear_commit_marks(commit, ALL_REV_FLAGS); } diff --git a/transport.c b/transport.c index 50f5830eb6b..95c1138e9ae 100644 --- a/transport.c +++ b/transport.c @@ -148,8 +148,10 @@ static struct ref *get_refs_from_bundle(struct transport *transport, for (i = 0; i < data->header.references.nr; i++) { struct ref_list_entry *e = data->header.references.list + i; - struct ref *ref = alloc_ref(e->name); - oidcpy(&ref->old_oid, &e->oid); + const char *name = e->name; + struct ref *ref = alloc_ref(name); + struct object_id *oid = &e->oid; + oidcpy(&ref->old_oid, oid); ref->next = result; result = ref; } From patchwork Fri Jul 2 09:57:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12355787 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89669C11F69 for ; Fri, 2 Jul 2021 09:57:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 68B3761413 for ; Fri, 2 Jul 2021 09:57:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231386AbhGBKAM (ORCPT ); Fri, 2 Jul 2021 06:00:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37220 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231357AbhGBKAL (ORCPT ); Fri, 2 Jul 2021 06:00:11 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ACF59C061764 for ; Fri, 2 Jul 2021 02:57:39 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id a13so11714852wrf.10 for ; Fri, 02 Jul 2021 02:57:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ttfUWHr8TA+l03bNVu3UXsPa8eRI6PJ5a7kXX5niVRE=; b=NIeWrbgRL0byZzdRCJWj+sC5ieIY2JhoRAAKDOvDyQG7a9eUu1c4DDtGBC923ikOIC R+Y1RSYPmsyjptgDPOHS1rEDXFtiMf787gqT6vZICs6gSvM12bzRhIWZenI08TP3+PaO LXDR44+JcpeSSJOn716GVDXeLdgjjGuDMqnavjMt3ggHvwJu561YLFb0UerClipYVo8o Q5gh308VUle1sk8H+wzpNNTF4ixxqTtOyUBAkX/WeU+/yCtkFr3MkHnrZ8vlq0oQRQWe dyBu2PR8miJ1OAdypNuI3vvIiTp2kVDOxk5LmFS/1igtuxBYXZXOVI6DizJLQbAjafvM HiqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ttfUWHr8TA+l03bNVu3UXsPa8eRI6PJ5a7kXX5niVRE=; b=R4D3dBIxfanPs9DQIhdqXSgoy//8V9QGSzYVn+gFne2zUhiGg7UoYCUEGU+YcCCa+k F0MvAM8uR93DB8iyqPZQwD0j4WX89wshP0aIttRONyqiIRN37saVllrVa8FhUgp+MQt9 cgE6psp8yDWB55WSkqeXufIJLfqsF2e4QPVm7BfVhVNxYaOUe02pIzX+qwDaeMzbEEVN HhTST6q4+QnyFRzioEkAaiCAc3y1EQ6+HNH+y0F3vVh6CCkM8DnVvNCNRS2TtIH9EEd8 I9nQ+dJn2OCZ80mrmlaDWZIIZYbg/dERfRUfu5pf1id+9G9DSCOOnXkoqAzmykuEMJhN GqwQ== X-Gm-Message-State: AOAM533g4ONnDgaCG2sfclT//OXkMGZPESV9z07z1ULQLPJC7uBzfOie m9GpIFtxgi48ShSL1JLwESb69Cgg8Ha8nw== X-Google-Smtp-Source: ABdhPJyEuaF9MS+FW+bTiI5KcaUe6WDL7iVGg5yVRYun2EPal8BL3jtiGV+acHieMVsb8rTaH3JxMg== X-Received: by 2002:a5d:4f12:: with SMTP id c18mr4899698wru.56.1625219858053; Fri, 02 Jul 2021 02:57:38 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id m6sm3307713wrw.9.2021.07.02.02.57.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Jul 2021 02:57:37 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , Andrei Rybak , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFy?= =?utf-8?b?bWFzb24=?= Subject: [PATCH v4 3/3] bundle: remove "ref_list" in favor of string-list.c API Date: Fri, 2 Jul 2021 11:57:32 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.632.g49a94b9226d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Move away from the "struct ref_list" in bundle.c in favor of the almost identical string-list.c API. That API fits this use-case perfectly, but did not exist in its current form when this code was added in 2e0afafebd (Add git-bundle: move objects and references by archive, 2007-02-22), with hindsight we could have used the path-list API, which later got renamed to string-list. See 8fd2cb4069 (Extract helper bits from c-merge-recursive work, 2006-07-25) We need to change "name" to "string" and "oid" to "util" to make this conversion, but other than that the APIs are pretty much identical for what bundle.c made use of. Let's also replace the memset(..,0,...) pattern with a more idiomatic "INIT" macro, and finally add a *_release() function so to free the allocated memory. Before this the add_to_ref_list() would leak memory, now e.g. "bundle list-heads" reports no memory leaks at all under valgrind. In the bundle_header_init() function we're using a clever trick to memcpy() what we'd get from the corresponding BUNDLE_HEADER_INIT. There is a concurrent series to make use of that pattern more generally, see [1]. 1. https://lore.kernel.org/git/cover-0.5-00000000000-20210701T104855Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/bundle.c | 12 +++++------ bundle.c | 52 ++++++++++++++++++++++++++---------------------- bundle.h | 21 +++++++++---------- transport.c | 8 +++++--- 4 files changed, 50 insertions(+), 43 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index 15e2bd61965..053a51bea1b 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -100,7 +100,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { } static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { - struct bundle_header header; + struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int quiet = 0; int ret; @@ -115,7 +115,6 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { builtin_bundle_verify_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ - memset(&header, 0, sizeof(header)); if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { ret = 1; goto cleanup; @@ -130,11 +129,12 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { ret = 0; cleanup: free(bundle_file); + bundle_header_release(&header); return ret; } static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) { - struct bundle_header header; + struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int ret; struct option options[] = { @@ -146,7 +146,6 @@ static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix builtin_bundle_list_heads_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ - memset(&header, 0, sizeof(header)); if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { ret = 1; goto cleanup; @@ -155,11 +154,12 @@ static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix ret = !!list_bundle_refs(&header, argc, argv); cleanup: free(bundle_file); + bundle_header_release(&header); return ret; } static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) { - struct bundle_header header; + struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int ret; struct option options[] = { @@ -171,7 +171,6 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) builtin_bundle_unbundle_usage, options, &bundle_file); /* bundle internals use argv[1] as further parameters */ - memset(&header, 0, sizeof(header)); if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { ret = 1; goto cleanup; @@ -180,6 +179,7 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) die(_("Need a repository to unbundle.")); ret = !!unbundle(the_repository, &header, bundle_fd, 0) || list_bundle_refs(&header, argc, argv); + bundle_header_release(&header); cleanup: free(bundle_file); return ret; diff --git a/bundle.c b/bundle.c index 7210e5e7105..ab63f402261 100644 --- a/bundle.c +++ b/bundle.c @@ -23,13 +23,16 @@ static struct { { 3, v3_bundle_signature }, }; -static void add_to_ref_list(const struct object_id *oid, const char *name, - struct ref_list *list) +void bundle_header_init(struct bundle_header *header) { - ALLOC_GROW(list->list, list->nr + 1, list->alloc); - oidcpy(&list->list[list->nr].oid, oid); - list->list[list->nr].name = xstrdup(name); - list->nr++; + struct bundle_header blank = BUNDLE_HEADER_INIT; + memcpy(header, &blank, sizeof(*header)); +} + +void bundle_header_release(struct bundle_header *header) +{ + string_list_clear(&header->prerequisites, 1); + string_list_clear(&header->references, 1); } static int parse_capability(struct bundle_header *header, const char *capability) @@ -112,10 +115,11 @@ static int parse_bundle_header(int fd, struct bundle_header *header, status = -1; break; } else { + struct object_id *dup = oiddup(&oid); if (is_prereq) - add_to_ref_list(&oid, "", &header->prerequisites); + string_list_append(&header->prerequisites, "")->util = dup; else - add_to_ref_list(&oid, p + 1, &header->references); + string_list_append(&header->references, p + 1)->util = dup; } } @@ -139,19 +143,19 @@ int read_bundle_header(const char *path, struct bundle_header *header) int is_bundle(const char *path, int quiet) { - struct bundle_header header; + struct bundle_header header = BUNDLE_HEADER_INIT; int fd = open(path, O_RDONLY); if (fd < 0) return 0; - memset(&header, 0, sizeof(header)); fd = parse_bundle_header(fd, &header, quiet ? NULL : path); if (fd >= 0) close(fd); + bundle_header_release(&header); return (fd >= 0); } -static int list_refs(struct ref_list *r, int argc, const char **argv) +static int list_refs(struct string_list *r, int argc, const char **argv) { int i; @@ -162,14 +166,14 @@ static int list_refs(struct ref_list *r, int argc, const char **argv) if (argc > 1) { int j; for (j = 1; j < argc; j++) - if (!strcmp(r->list[i].name, argv[j])) + if (!strcmp(r->items[i].string, argv[j])) break; if (j == argc) continue; } - oid = &r->list[i].oid; - name = r->list[i].name; + oid = r->items[i].util; + name = r->items[i].string; printf("%s %s\n", oid_to_hex(oid), name); } return 0; @@ -186,7 +190,7 @@ int verify_bundle(struct repository *r, * Do fast check, then if any prereqs are missing then go line by line * to be verbose about the errors */ - struct ref_list *p = &header->prerequisites; + struct string_list *p = &header->prerequisites; struct rev_info revs; const char *argv[] = {NULL, "--all", NULL}; struct commit *commit; @@ -198,9 +202,9 @@ int verify_bundle(struct repository *r, repo_init_revisions(r, &revs, NULL); for (i = 0; i < p->nr; i++) { - struct ref_list_entry *e = p->list + i; - const char *name = e->name; - struct object_id *oid = &e->oid; + struct string_list_item *e = p->items + i; + const char *name = e->string; + struct object_id *oid = e->util; struct object *o = parse_object(r, oid); if (o) { o->flags |= PREREQ_MARK; @@ -225,9 +229,9 @@ int verify_bundle(struct repository *r, i--; for (i = 0; i < p->nr; i++) { - struct ref_list_entry *e = p->list + i; - const char *name = e->name; - struct object_id *oid = &e->oid; + struct string_list_item *e = p->items + i; + const char *name = e->string; + const struct object_id *oid = e->util; struct object *o = parse_object(r, oid); assert(o); /* otherwise we'd have returned early */ if (o->flags & SHOWN) @@ -239,15 +243,15 @@ int verify_bundle(struct repository *r, /* Clean up objects used, as they will be reused. */ for (i = 0; i < p->nr; i++) { - struct ref_list_entry *e = p->list + i; - struct object_id *oid = &e->oid; + struct string_list_item *e = p->items + i; + struct object_id *oid = e->util; commit = lookup_commit_reference_gently(r, oid, 1); if (commit) clear_commit_marks(commit, ALL_REV_FLAGS); } if (verbose) { - struct ref_list *r; + struct string_list *r; r = &header->references; printf_ln(Q_("The bundle contains this ref:", diff --git a/bundle.h b/bundle.h index f9e2d1c8ef5..1927d8cd6a4 100644 --- a/bundle.h +++ b/bundle.h @@ -3,22 +3,23 @@ #include "strvec.h" #include "cache.h" - -struct ref_list { - unsigned int nr, alloc; - struct ref_list_entry { - struct object_id oid; - char *name; - } *list; -}; +#include "string-list.h" struct bundle_header { unsigned version; - struct ref_list prerequisites; - struct ref_list references; + struct string_list prerequisites; + struct string_list references; const struct git_hash_algo *hash_algo; }; +#define BUNDLE_HEADER_INIT \ +{ \ + .prerequisites = STRING_LIST_INIT_DUP, \ + .references = STRING_LIST_INIT_DUP, \ +} +void bundle_header_init(struct bundle_header *header); +void bundle_header_release(struct bundle_header *header); + int is_bundle(const char *path, int quiet); int read_bundle_header(const char *path, struct bundle_header *header); int create_bundle(struct repository *r, const char *path, diff --git a/transport.c b/transport.c index 95c1138e9ae..745ffa22474 100644 --- a/transport.c +++ b/transport.c @@ -147,10 +147,10 @@ static struct ref *get_refs_from_bundle(struct transport *transport, transport->hash_algo = data->header.hash_algo; for (i = 0; i < data->header.references.nr; i++) { - struct ref_list_entry *e = data->header.references.list + i; - const char *name = e->name; + struct string_list_item *e = data->header.references.items + i; + const char *name = e->string; struct ref *ref = alloc_ref(name); - struct object_id *oid = &e->oid; + struct object_id *oid = e->util; oidcpy(&ref->old_oid, oid); ref->next = result; result = ref; @@ -177,6 +177,7 @@ static int close_bundle(struct transport *transport) struct bundle_transport_data *data = transport->data; if (data->fd > 0) close(data->fd); + bundle_header_release(&data->header); free(data); return 0; } @@ -1083,6 +1084,7 @@ struct transport *transport_get(struct remote *remote, const char *url) die(_("git-over-rsync is no longer supported")); } else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) { struct bundle_transport_data *data = xcalloc(1, sizeof(*data)); + bundle_header_init(&data->header); transport_check_allowed("file"); ret->data = data; ret->vtable = &bundle_vtable;