From patchwork Thu Aug 13 15:55:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 11712635 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5AC33138C for ; Thu, 13 Aug 2020 15:55:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4135D20791 for ; Thu, 13 Aug 2020 15:55:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726772AbgHMPzB (ORCPT ); Thu, 13 Aug 2020 11:55:01 -0400 Received: from cloud.peff.net ([104.130.231.41]:57884 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726249AbgHMPzB (ORCPT ); Thu, 13 Aug 2020 11:55:01 -0400 Received: (qmail 20315 invoked by uid 109); 13 Aug 2020 15:55:01 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 13 Aug 2020 15:55:01 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 15095 invoked by uid 111); 13 Aug 2020 15:55:00 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Aug 2020 11:55:00 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 13 Aug 2020 11:55:00 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 1/2] stop calling UNLEAK() before die() Message-ID: <20200813155500.GA897132@coredump.intra.peff.net> References: <20200813155426.GA896769@coredump.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813155426.GA896769@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The point of UNLEAK() is to make a reference to a variable that is about to go out of scope so that leak-checkers will consider it to be not-leaked. Doing so right before die() is therefore pointless; even though we are about to exit the program, the variable will still be on the stack and accessible to leak-checkers. These annotations aren't really hurting anything, but they clutter the code and set a bad example of how to use UNLEAK(). Signed-off-by: Jeff King --- bugreport.c | 4 +--- midx.c | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/bugreport.c b/bugreport.c index 09579e268d..7ca0fba1b8 100644 --- a/bugreport.c +++ b/bugreport.c @@ -175,10 +175,8 @@ int cmd_main(int argc, const char **argv) /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666); - if (report < 0) { - UNLEAK(report_path); + if (report < 0) die(_("couldn't create a new file at '%s'"), report_path.buf); - } if (write_in_full(report, buffer.buf, buffer.len) < 0) die_errno(_("unable to write to %s"), report_path.buf); diff --git a/midx.c b/midx.c index a5fb797ede..737420f157 100644 --- a/midx.c +++ b/midx.c @@ -807,11 +807,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * int result = 0; midx_name = get_midx_filename(object_dir); - if (safe_create_leading_directories(midx_name)) { - UNLEAK(midx_name); + if (safe_create_leading_directories(midx_name)) die_errno(_("unable to create leading directories of %s"), midx_name); - } if (m) packs.m = m; @@ -1051,10 +1049,8 @@ void clear_midx_file(struct repository *r) r->objects->multi_pack_index = NULL; } - if (remove_path(midx)) { - UNLEAK(midx); + if (remove_path(midx)) die(_("failed to clear multi-pack-index at %s"), midx); - } free(midx); } From patchwork Thu Aug 13 15:55:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 11712637 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CF127138C for ; Thu, 13 Aug 2020 15:55:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C092A20791 for ; Thu, 13 Aug 2020 15:55:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726522AbgHMPzx (ORCPT ); Thu, 13 Aug 2020 11:55:53 -0400 Received: from cloud.peff.net ([104.130.231.41]:57890 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726167AbgHMPzx (ORCPT ); Thu, 13 Aug 2020 11:55:53 -0400 Received: (qmail 20322 invoked by uid 109); 13 Aug 2020 15:55:52 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 13 Aug 2020 15:55:52 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 15122 invoked by uid 111); 13 Aug 2020 15:55:52 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Aug 2020 11:55:52 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 13 Aug 2020 11:55:51 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 2/2] ls-remote: simplify UNLEAK() usage Message-ID: <20200813155551.GB897132@coredump.intra.peff.net> References: <20200813155426.GA896769@coredump.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813155426.GA896769@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We UNLEAK() the "sorting" list created by parsing command-line options (which is essentially used until the program exits). But we do so right before leaving the cmd_ls_remote() function, which means we have to hit all of the exits. But the point of UNLEAK() is that it's an annotation which doesn't impact the variable itself. We can mark it as soon as we're done writing its value, and then we only have to do so once. This gives us a minor code reduction, and serves as a better example of how UNLEAK() can be used. Signed-off-by: Jeff King --- builtin/ls-remote.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index ea91679f33..092917eca2 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -83,6 +83,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) PARSE_OPT_STOP_AT_NON_OPTION); dest = argv[0]; + UNLEAK(sorting); + if (argc > 1) { int i; pattern = xcalloc(argc, sizeof(const char *)); @@ -107,7 +109,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) if (get_url) { printf("%s\n", *remote->url); - UNLEAK(sorting); return 0; } @@ -122,10 +123,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport)); repo_set_hash_algo(the_repository, hash_algo); } - if (transport_disconnect(transport)) { - UNLEAK(sorting); + if (transport_disconnect(transport)) return 1; - } if (!dest && !quiet) fprintf(stderr, "From %s\n", *remote->url); @@ -150,7 +149,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) status = 0; /* we found something */ } - UNLEAK(sorting); ref_array_clear(&ref_array); return status; }