From patchwork Thu Jun 17 11:21:47 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: 12327547 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 8CBB1C2B9F4 for ; Thu, 17 Jun 2021 11:22:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 64B6B613BF for ; Thu, 17 Jun 2021 11:22:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232520AbhFQLYD (ORCPT ); Thu, 17 Jun 2021 07:24:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230269AbhFQLYB (ORCPT ); Thu, 17 Jun 2021 07:24:01 -0400 Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 08885C06175F for ; Thu, 17 Jun 2021 04:21:54 -0700 (PDT) Received: by mail-ed1-x531.google.com with SMTP id r7so3439458edv.12 for ; Thu, 17 Jun 2021 04:21:53 -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=ZcXxzf2L2wZCrJU8+zniLttskGhTcm/BqidTkJU7Rr4=; b=iiqgWPR5gryndhu+8XZdaUlMkESN0KGebSHipVhLsJ49HAcUCOmPfG5pDG2CZgG2tR lej9Tk4xQN7d83kwP7hRIZSOWiK9LrwlXJNv5XTc2kZihRhRvBq/R2AWogviw0j+b6nQ lKhLT7t703BxlVxcghi2kwHZNMmZdAR+eFxFXT6x8z+QK+e8W+ZrJteaejs/d6ztVOyi rVDk9CX15tRK89psT9FxWusJ6e6PI0mVcu2iTh3L2X/pEewrMGGjqp8yxQds7VQgzsZi +zMvDBseteOzccHW0ldDwmDUFvnwm7o4zytpUDCXkJq99EprGYgX5I4YTkJ1M6yoETH3 0Tfw== 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=ZcXxzf2L2wZCrJU8+zniLttskGhTcm/BqidTkJU7Rr4=; b=skLWN2EvuAUviwDDk+0AfuqqpkyFIKoJ5lbVXHXjazeoEJOZU4312fTQN4P19atK// o+crgj0TUHsyajeo6Oy8tXXPCCEW1iG5L62c+vzaJblKRTfG4EpAqAh+5idT6mqYl8/7 JXzHOKGw52Ld0NXVoAx6VOryaafjYf+IZducea+Fm6hadHX6Hbk4eo4eMkAQKIRPh+uU bj4b8oHwTzrTgczd4W/h/fS7wGws6G77yQTaPCImRUdRtdWbZ27ikjpxgRl2H5IQHPDe 3KKIY3mDSgSr6rtGe52HP1MqpaYID7OSl2qx5yuQG6w4Mfr4zDeFbg3vnU0Pt/dTkHwj /eeg== X-Gm-Message-State: AOAM532JyWXesqxEQ7EGiHa8nQAeTDByKVkd4Do+/8t1X0MXuLWZY9GR q08UeQ8SMWwRczmu3If0rbyMIhJX+a1URw== X-Google-Smtp-Source: ABdhPJwPFkBl+gtl4gRzU38WWD9a/viM65Xs8Ap8vBucastsfPmyBkBDVdWScMN0umP6VphS2FaIMg== X-Received: by 2002:a05:6402:6d1:: with SMTP id n17mr5825952edy.116.1623928912419; Thu, 17 Jun 2021 04:21:52 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id cw10sm3583153ejb.62.2021.06.17.04.21.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 04:21:51 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , =?utf-8?b?w4Z2YXIgQXJu?= =?utf-8?b?ZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 1/3] bundle cmd: stop leaking memory from parse_options_cmd_bundle() Date: Thu, 17 Jun 2021 13:21:47 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.571.gdba276db2c 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. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/bundle.c | 79 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index ea6948110b..7778297277 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,8 @@ 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 die_no_repo = 0; + int ret; struct option options[] = { OPT_SET_INT('q', "quiet", &progress, N_("do not show progress meter"), 0), @@ -76,7 +77,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); @@ -92,77 +93,107 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { if (progress && all_progress_implied) strvec_push(&pack_opts, "--all-progress-implied"); - if (!startup_info->have_repository) + if (!startup_info->have_repository) { + die_no_repo = 1; + goto cleanup; + } + ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version); +cleanup: + free(bundle_file); + if (die_no_repo) die(_("Need a repository to create a bundle.")); - return !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version); + 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 die_no_repo = 0; + 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 (!startup_info->have_repository) - die(_("Need a repository to unbundle.")); - return !!unbundle(the_repository, &header, bundle_fd, 0) || + if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) { + ret = 1; + goto cleanup; + } + if (!startup_info->have_repository) { + die_no_repo = 1; + goto cleanup; + } + ret = !!unbundle(the_repository, &header, bundle_fd, 0) || list_bundle_refs(&header, argc, argv); +cleanup: + if (die_no_repo) + die(_("Need a repository to unbundle.")); + free(bundle_file); + return ret; } int cmd_bundle(int argc, const char **argv, const char *prefix) From patchwork Thu Jun 17 11:21:48 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: 12327553 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 052BDC49361 for ; Thu, 17 Jun 2021 11:22:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D6ACC60FF0 for ; Thu, 17 Jun 2021 11:22:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232579AbhFQLY1 (ORCPT ); Thu, 17 Jun 2021 07:24:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232523AbhFQLYE (ORCPT ); Thu, 17 Jun 2021 07:24:04 -0400 Received: from mail-ej1-x636.google.com (mail-ej1-x636.google.com [IPv6:2a00:1450:4864:20::636]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D969DC06175F for ; Thu, 17 Jun 2021 04:21:54 -0700 (PDT) Received: by mail-ej1-x636.google.com with SMTP id nd37so9289814ejc.3 for ; Thu, 17 Jun 2021 04:21:54 -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=xEx3RpZGqNrwrSLJU5DUM8Kbmf3BAvF6Ko6IpNNbIew=; b=h+Y//hFZfzMRsdVns0X9Gm5fqhZ0P08lkqpkLeB+E/JnPy9wDQhvZJ4+W94LTvUqQD RLkhE3rIHU79xrWVOyScRXuVnlWkgpoYib/KQ8uQqzt/v98b+rgM3eKup9G/H2H1jdWm e4PMVPlRVi9rH8iqHYcI+NeA6OFPZt522as4PvtUZsUG6jc5H3UTxTXbj436OdaexO8V n14dd7DD55/M+GQ+9sPctII8fasFMb9PQyJCe3qblW7+RTj1KZtafDO/hRTwKXKvEwXK j8JZSdlD3gr8LNCxfA9zUihOfJF2xP5HkN1yjmwFyueNnZ72F9rmdKrgwE8DiDSB/e2r 1Tlw== 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=xEx3RpZGqNrwrSLJU5DUM8Kbmf3BAvF6Ko6IpNNbIew=; b=CIV0/30H3NMiIYtC0lfQosRE1LzFR59WUZSshGl++WrMT4YoJwuQSCwUmn0/s+F3R6 4nNWlgp5LIUrL36K4TrVMzYUh4PSMqwpaXDbuugf3hgekR3QBdajtCX/iqV9MykWzPTM 3lSi1QNNjN+GTEDte02cUGNYRFTa/Zy66Vat4iPXBuYbPvVHgDX/PzUi3BVUF/tFuCC2 Z5G6NUkVkG6Dvl4WoBcB3ES5pbopaWqOsV4kmc3Yq9FkJwZMLAPcRvZD6iYHvXhmiTiZ ELabEuckQCOiFSME71/DTlzhwJ9SnHr65zPn2hgqaxz2Q4m+KYen5h0FYtTvTJXuN5kj Qbmw== X-Gm-Message-State: AOAM531sJ7onQ/BZD/bfffp+WS5k5coafybaZvFPmDT/6o4XGaqamcdi F87Hn64JrZ7jBA0fAvx4HWJ75irNcoNJpw== X-Google-Smtp-Source: ABdhPJwXCDO+SzHbULyGlqbIgHeuUCL6zYTDUAyoUYlfnCAxugrXvGJCj+0JoSk0sPq7vxmrMzv4XA== X-Received: by 2002:a17:906:3057:: with SMTP id d23mr4635820ejd.131.1623928913215; Thu, 17 Jun 2021 04:21:53 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id cw10sm3583153ejb.62.2021.06.17.04.21.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 04:21:52 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , =?utf-8?b?w4Z2YXIgQXJu?= =?utf-8?b?ZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 2/3] bundle.c: use a temporary variable for OIDs and names Date: Thu, 17 Jun 2021 13:21:48 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.571.gdba276db2c 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 | 22 +++++++++++++++------- transport.c | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/bundle.c b/bundle.c index 693d619551..621708f40e 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,7 +199,8 @@ 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); + 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); @@ -202,7 +208,7 @@ int verify_bundle(struct repository *r, } if (++ret == 1) error("%s", message); - error("%s %s", oid_to_hex(&e->oid), e->name); + error("%s %s", oid_to_hex(oid), e->name); } if (revs.pending.nr != p->nr) return ret; @@ -219,19 +225,21 @@ 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); + 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), e->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 50f5830eb6..9d601c8c95 100644 --- a/transport.c +++ b/transport.c @@ -149,7 +149,8 @@ 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); + struct object_id *oid = &e->oid; + oidcpy(&ref->old_oid, oid); ref->next = result; result = ref; } From patchwork Thu Jun 17 11:21:49 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: 12327551 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 551BAC2B9F4 for ; Thu, 17 Jun 2021 11:22:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3549B60FF0 for ; Thu, 17 Jun 2021 11:22:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232572AbhFQLYV (ORCPT ); Thu, 17 Jun 2021 07:24:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232524AbhFQLYE (ORCPT ); Thu, 17 Jun 2021 07:24:04 -0400 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9D92EC061760 for ; Thu, 17 Jun 2021 04:21:55 -0700 (PDT) Received: by mail-ej1-x632.google.com with SMTP id og14so9258544ejc.5 for ; Thu, 17 Jun 2021 04:21:55 -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=uspAcrCzFhb91aXCTI0FjE/dOddzso27Y+fQIsqrBVc=; b=OTNw2vNtJb3N0s7FB0C/tMCEWO4d4acPvcZT0cKkFR8oO84dE/BFvjHBTarKAKwe26 /xFjVhvixeT4Wh3IEwwxVxqpXzpWYof2cbDmm+LlQC6SCjbED8X/3hpQnU3bV6JKYB6C yTgTp7KIFn1vOm2qNXeVUN0P1eLS/MI90TQVT9VcgyAUjzXLbCacTg5PNMLQ27aGAxu8 esAL9nf0Yvxq4f2ZHt7p2JHwBMt1ykdrEaT3UCGL9N/xAlyJQJ1AFVwpkme08FbwWDd8 tyxUFLoFa39/osVm2jjUv2lRrdopthXcZZTML5/68FS0Em1UMT0lUVieNXxvvnQgmz3D BLkw== 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=uspAcrCzFhb91aXCTI0FjE/dOddzso27Y+fQIsqrBVc=; b=pmO6TnZwZVl+ySWqDbZO3EzyE/EAKuWppPIqWZVi6pA1V550JzJGqIiC7Xfewme00A ZlQFKiuPw5YtEzE6U2DlRUf4n48iJNnKuOwLXXIyBtzhl6E8G3z/O7RsKU2IdvqczzPf vTi0l+78T2lJLpIEoisUAaYogG0Ctu/JqpB7ZjMWdjJ2nh0qguLDMhsjGIO2PkDLYXnT nRxYjJ2Uo4T9ZpVgNH8vzvlxlKaxAra7541LIviP2LNyOq7FMEe5NURs6Ng6GFojPtRV +CsL+GiC48P1EirARLMkwn4+cF3UIzqApvZSJD8uEUNrqVwS653eYDcqS1NCH3MtTqaW YS2w== X-Gm-Message-State: AOAM5324mXZZze2vkiv1FrwhE7+lMKb8+b0vxgGFW2+LKJvrwVbGE4PJ jjU0dlHfyH8RHJibBgMNtK9svY2Gg9ADLg== X-Google-Smtp-Source: ABdhPJyELNMtsmCQt1F4ppnP8Iv2m3Evu4/OvDc/vcpcFvfkaWCDRvQ3zlRztt6oYUtXjq6UnaUm6Q== X-Received: by 2002:a17:907:9fd:: with SMTP id ce29mr4535369ejc.396.1623928913984; Thu, 17 Jun 2021 04:21:53 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id cw10sm3583153ejb.62.2021.06.17.04.21.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 04:21:53 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Johannes Schindelin , =?utf-8?b?w4Z2YXIgQXJu?= =?utf-8?b?ZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 3/3] bundle: remove "ref_list" in favor of string-list.c API Date: Thu, 17 Jun 2021 13:21:49 +0200 Message-Id: X-Mailer: git-send-email 2.32.0.571.gdba276db2c 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. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/bundle.c | 12 ++++----- bundle.c | 64 ++++++++++++++++++++++++------------------------ bundle.h | 20 +++++++-------- transport.c | 10 +++++--- 4 files changed, 55 insertions(+), 51 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index 7778297277..8d82255280 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -106,7 +106,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; @@ -121,7 +121,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; @@ -136,11 +135,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[] = { @@ -152,7 +152,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; @@ -161,11 +160,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 die_no_repo = 0; int ret; @@ -178,7 +178,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; @@ -189,6 +188,7 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) } ret = !!unbundle(the_repository, &header, bundle_fd, 0) || list_bundle_refs(&header, argc, argv); + bundle_header_release(&header); cleanup: if (die_no_repo) die(_("Need a repository to unbundle.")); diff --git a/bundle.c b/bundle.c index 621708f40e..d36eeee1a5 100644 --- a/bundle.c +++ b/bundle.c @@ -23,15 +23,6 @@ static struct { { 3, v3_bundle_signature }, }; -static void add_to_ref_list(const struct object_id *oid, const char *name, - struct ref_list *list) -{ - 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++; -} - static int parse_capability(struct bundle_header *header, const char *capability) { const char *arg; @@ -79,7 +70,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, /* The bundle header ends with an empty line */ while (!strbuf_getwholeline_fd(&buf, fd, '\n') && buf.len && buf.buf[0] != '\n') { - struct object_id oid; + struct object_id *oid; int is_prereq = 0; const char *p; @@ -103,19 +94,22 @@ static int parse_bundle_header(int fd, struct bundle_header *header, * Prerequisites have object name that is optionally * followed by SP and subject line. */ - if (parse_oid_hex_algop(buf.buf, &oid, &p, header->hash_algo) || + oid = xmalloc(sizeof(struct object_id)); + if (parse_oid_hex_algop(buf.buf, oid, &p, header->hash_algo) || (*p && !isspace(*p)) || (!is_prereq && !*p)) { if (report_path) error(_("unrecognized header: %s%s (%d)"), (is_prereq ? "-" : ""), buf.buf, (int)buf.len); status = -1; + free(oid); break; } else { - if (is_prereq) - add_to_ref_list(&oid, "", &header->prerequisites); - else - add_to_ref_list(&oid, p + 1, &header->references); + const char *string = is_prereq ? "" : p + 1; + struct string_list *list = is_prereq + ? &header->prerequisites + : &header->references; + string_list_append(list, string)->util = oid; } } @@ -139,19 +133,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 +156,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 +180,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,17 +192,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_id *oid = &e->oid; + struct string_list_item *e = p->items + i; + struct object_id *oid = e->util; 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, e->string); continue; } if (++ret == 1) error("%s", message); - error("%s %s", oid_to_hex(oid), e->name); + error("%s %s", oid_to_hex(oid), e->string); } if (revs.pending.nr != p->nr) return ret; @@ -224,28 +218,28 @@ int verify_bundle(struct repository *r, i--; 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; + 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) continue; if (++ret == 1) error("%s", message); - error("%s %s", oid_to_hex(oid), e->name); + error("%s %s", oid_to_hex(oid), e->string); } /* 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:", @@ -582,3 +576,9 @@ int unbundle(struct repository *r, struct bundle_header *header, return error(_("index-pack died")); return 0; } + +void bundle_header_release(struct bundle_header *header) +{ + string_list_clear(&header->prerequisites, 1); + string_list_clear(&header->references, 1); +} diff --git a/bundle.h b/bundle.h index f9e2d1c8ef..f98c1d24d9 100644 --- a/bundle.h +++ b/bundle.h @@ -3,22 +3,21 @@ #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, \ +} + 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, @@ -30,5 +29,6 @@ int unbundle(struct repository *r, struct bundle_header *header, int bundle_fd, int flags); int list_bundle_refs(struct bundle_header *header, int argc, const char **argv); +void bundle_header_release(struct bundle_header *header); #endif diff --git a/transport.c b/transport.c index 9d601c8c95..667c4e0cc6 100644 --- a/transport.c +++ b/transport.c @@ -147,13 +147,14 @@ 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; - struct ref *ref = alloc_ref(e->name); - struct object_id *oid = &e->oid; + struct string_list_item *e = data->header.references.items + i; + struct ref *ref = alloc_ref(e->string); + const struct object_id *oid = e->util; oidcpy(&ref->old_oid, oid); ref->next = result; result = ref; } + string_list_clear(&data->header.references, 1); return result; } @@ -176,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; } @@ -1082,6 +1084,8 @@ 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)); + string_list_init(&data->header.prerequisites, 1); + string_list_init(&data->header.references, 1); transport_check_allowed("file"); ret->data = data; ret->vtable = &bundle_vtable;