From patchwork Thu Nov 25 22:52:16 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: 12639935 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A6791C433F5 for ; Thu, 25 Nov 2021 22:54:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238116AbhKYW5o (ORCPT ); Thu, 25 Nov 2021 17:57:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57554 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235150AbhKYWzm (ORCPT ); Thu, 25 Nov 2021 17:55:42 -0500 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B75CC06173E for ; Thu, 25 Nov 2021 14:52:30 -0800 (PST) Received: by mail-wr1-x435.google.com with SMTP id l16so14594782wrp.11 for ; Thu, 25 Nov 2021 14:52:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=jW4wAt/AkW1rBXADqj0gqMV7SlS31UouBJBUkvPxdro=; b=PgZoLvo0oFzFxSwLsDrnaZAr1Ne2j+WhykG+pQPu6rtgMGjtF81r+ZDoCLDc/algaA SWRuyy5khaipE/jLmr9m0TPsH4aeLbfNDiXYxGJ+B5ADHfQLG6sCjzKzWBtqZ5exwqim RvRUpYqdy40v5jZzjbhG48mmwDSZTrnysBXWuSRmL7dWLTWvSS6eGj3aM4DP1PnaPdix 74+s9XjN6/9hmeI8IJY/FYAs900yCpWMfu66jLv+kPumBI8cz7HS23sCmT5l+RDghiT6 Z0Csc0JZXMNdhLO9gkqpBzHpr4NqweecllqBBK7WtOUQB6irySThwSNN1472b4R8wVsq IK/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=jW4wAt/AkW1rBXADqj0gqMV7SlS31UouBJBUkvPxdro=; b=0AohEaDU4+ql47fClILfPZ1/o+ltlkQjrtAU8YBFn/8G3AnJx59rF98pGAbjC3Ohyf 1tXftJgqqGkZkGJML8jz9aX0mZ+IBDBKd8AVgoCx3aqqwGXC9Hzek0fGPJzayVEL3fFa A34JQQ92nbXuMPlQnKXlrK8kQCUQYYEeNBc4w0jIBaGTrEJlgGlJvo+yP66xedxeiBzl AciqvyflYDfahjWQ5a4NSygOX7UBGr4LebwTh0hQznIMVuHhwkDM2vY0i2Ebl5EQkq0d kuFqjRJYe4luQGh+0U7Ls3pFKEPplyp+/C6pQHfQNu4+n6a0tqz5jCJ6MAOHjETPBKDo 12Xw== X-Gm-Message-State: AOAM530hHYIYzu4kcE+Wf6iEkbSstzqAgqPQabszk4GqNxid9F4ghat5 eJ2mMkjVDt4KA8SF6zGg0CZhuJ8/ydRZoA== X-Google-Smtp-Source: ABdhPJz6JpmH0OEr5RyMNGO7h6P142Av0PPyvQEZxokbroldE8sGAaAxldCicyzP2fg7nuNHP1cFew== X-Received: by 2002:adf:8bd2:: with SMTP id w18mr10027658wra.557.1637880748769; Thu, 25 Nov 2021 14:52:28 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:27 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 1/9] worktree: stop being overly intimate with run_command() internals Date: Thu, 25 Nov 2021 23:52:16 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine add_worktree() reuses a `child_process` for three run_command() invocations, but to do so, it has overly-intimate knowledge of run-command.c internals. In particular, it knows that it must reset child_process::argv to NULL for each subsequent invocation[*] in order for start_command() to latch the newly-populated child_process::args for each invocation, even though this behavior is not a part of the documented API. Beyond having overly-intimate knowledge of run-command.c internals, the reuse of one `child_process` for three run_command() invocations smells like an unnecessary micro-optimization. Therefore, stop sharing one `child_process` and instead use a new one for each run_command() call. [*] If child_process::argv is not reset to NULL, then subsequent run_command() invocations will instead incorrectly access a dangling pointer to freed memory which had been allocated by child_process::args on the previous run. This is due to the following code in start_command(): if (!cmd->argv) cmd->argv = cmd->args.v; Signed-off-by: Eric Sunshine Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/worktree.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d22ece93e1a..9edd3e2829b 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -355,8 +355,8 @@ static int add_worktree(const char *path, const char *refname, goto done; if (opts->checkout) { - cp.argv = NULL; - strvec_clear(&cp.args); + struct child_process cp = CHILD_PROCESS_INIT; + cp.git_cmd = 1; strvec_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL); if (opts->quiet) strvec_push(&cp.args, "--quiet"); @@ -385,12 +385,11 @@ static int add_worktree(const char *path, const char *refname, const char *hook = find_hook("post-checkout"); if (hook) { const char *env[] = { "GIT_DIR", "GIT_WORK_TREE", NULL }; - cp.git_cmd = 0; + struct child_process cp = CHILD_PROCESS_INIT; cp.no_stdin = 1; cp.stdout_to_stderr = 1; cp.dir = path; cp.env = env; - cp.argv = NULL; cp.trace2_hook_name = "post-checkout"; strvec_pushl(&cp.args, absolute_path(hook), oid_to_hex(null_oid()), From patchwork Thu Nov 25 22:52:17 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: 12639939 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF5F6C433EF for ; Thu, 25 Nov 2021 22:54:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239272AbhKYW5p (ORCPT ); Thu, 25 Nov 2021 17:57:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57560 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233653AbhKYWzn (ORCPT ); Thu, 25 Nov 2021 17:55:43 -0500 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 360C8C061746 for ; Thu, 25 Nov 2021 14:52:31 -0800 (PST) Received: by mail-wm1-x329.google.com with SMTP id p27-20020a05600c1d9b00b0033bf8532855so5617135wms.3 for ; Thu, 25 Nov 2021 14:52:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=TvQflBWFBqNZAJoD0HXbPiWOLNkmUz45gIFX8uTeCFo=; b=TXBiwo5o8I7e+Q1vR0Ycrjg8nuCrTDi6btulp8XyO4U2I8Bz7s1D/cOwJcBzhoON6P 3kdLFRenILc2kJG8KTHoILC5sEu3ccc0e9e6brKjZsLSdlf0rBFyfUeIzh672EzKR+7+ N4qcPr5loeHHmsQQ7DGhsYsp5q2mLup9AO8aNVZhYH3tOQoo4gRCW+qu/tJfo0AMAm6j bt7jKGEAQ6erX5tgbLIy5YL8bcM3qnSxT0+wR82Ba3MmKw0U1tgiXdyiB2VmxD87758Z vVJd0Vw6CWpQR3CzP269StVa+V/fRYm4lC2wh9W28ljvR6wdxgVQyTEpisSEZ3WhOMAT 0fHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=TvQflBWFBqNZAJoD0HXbPiWOLNkmUz45gIFX8uTeCFo=; b=ViyXsSc/SKJ4XDJi8TDlcZrCZmvodWqgKLzMaX2cCAEHb49k+r5Qbyvi29RNAlw0Jv ecsRjmWSq/iPlWM2xKK4WCnQVLxxlIVnoEdAJeiUY4eYAUsRkSZUWjw2baB7FKXXxpid 3BhqCVCHWS2WrNsQIvI2hMrqVndOdunNrrhy2o0rkik9F19FU4LFZv+86jaQiL3E1Eeh rdb6Tx3gNcDJxEIJBwVVjz3svQU5JcV/ftANLeDj9i7s4VhHH4IS0z07+ETNHe7cRUxu XGs7OrMAf6pG1SPGh5AtWR+yuc2e4Sw5gEJYMQs5SHTZ878u5VCkfJkhE888yNHtG4DN q9Og== X-Gm-Message-State: AOAM5305l7ekMuTu4zIZcpabwQO0zTf71vEkhi8RZXXUq+3XDd8BGgNO V5fnGPtFHEBBN2rEIjo0T6NdLLDrGBY+Pw== X-Google-Smtp-Source: ABdhPJzP3sQDOvFRsDNyylmQbYkwA+gioQ9Jc9avgf7Th7RtAZEdS+VPi2cI7p0P4D/Xeb872TODVA== X-Received: by 2002:a1c:7f43:: with SMTP id a64mr11616707wmd.133.1637880749613; Thu, 25 Nov 2021 14:52:29 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:29 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 2/9] upload-archive: use regular "struct child_process" pattern Date: Thu, 25 Nov 2021 23:52:17 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This pattern added [1] in seems to have been intentional, but since [2] and [3] we've wanted do initialization of what's now the "struct strvec" "args" and "env_array" members. Let's not trample on that initialization here. 1. 1bc01efed17 (upload-archive: use start_command instead of fork, 2011-11-19) 2. c460c0ecdca (run-command: store an optional argv_array, 2014-05-15) 3. 9a583dc39e (run-command: add env_array, an optional argv_array for env, 2014-10-19) Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/upload-archive.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c index 24654b4c9bf..98d028dae67 100644 --- a/builtin/upload-archive.c +++ b/builtin/upload-archive.c @@ -77,7 +77,7 @@ static ssize_t process_input(int child_fd, int band) int cmd_upload_archive(int argc, const char **argv, const char *prefix) { - struct child_process writer = { argv }; + struct child_process writer = CHILD_PROCESS_INIT; if (argc == 2 && !strcmp(argv[1], "-h")) usage(upload_archive_usage); @@ -89,9 +89,10 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix) * multiplexed out to our fd#1. If the child dies, we tell the other * end over channel #3. */ - argv[0] = "upload-archive--writer"; writer.out = writer.err = -1; writer.git_cmd = 1; + strvec_push(&writer.args, "upload-archive--writer"); + strvec_pushv(&writer.args, argv + 1); if (start_command(&writer)) { int err = errno; packet_write_fmt(1, "NACK unable to spawn subprocess\n"); From patchwork Thu Nov 25 22:52:18 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: 12639937 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B701AC433FE for ; Thu, 25 Nov 2021 22:54:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240668AbhKYW5q (ORCPT ); Thu, 25 Nov 2021 17:57:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236021AbhKYWzo (ORCPT ); Thu, 25 Nov 2021 17:55:44 -0500 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F2019C061748 for ; Thu, 25 Nov 2021 14:52:31 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id p18so6694898wmq.5 for ; Thu, 25 Nov 2021 14:52:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=3wgdmZfhhzumhcKNRCeztfFkGQJVvdnE8B2YjYg6egM=; b=fkCdL60sfXzFsJrTza6I9PLLC5N+Vypj3BMX9uNxHW5pTSPreziQ1k/h0RcoG3Ce+3 XfUxTc4m/pdU9hTQo1cjpKxNcKaxSjbKTSC39eMSv7GNL4771hOjd5aIE/i0vUnFqhKm fMDtlbHtK18UbCvi+BkTHZ1t/gQxdvfTC6K6Z1bhIBgmIX1K1tPkJaKwPhmUKDFHKdWz SL+2VMUGs7HiS36mMnTClC48RCZBDkYFGnvZR1weDJYO03LlZcCSfikGJauDnU42usLP D+YlesMEOu7bjYa0yNZrd1Toqvwp3eDoLRtkCLX6QmKvyCBinKkrksbr/TqYGJtq/GKP uF0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=3wgdmZfhhzumhcKNRCeztfFkGQJVvdnE8B2YjYg6egM=; b=SpHtc62RT8aiAFMUDTo7VNxAhaSvrmp/ejgTjJdoX732pYu9nJmkTmUo1tBoy+mT4z /9sofUnwSYSqbtHuJ5MaLadMSLUn06M3RxFH/PYRptOv/HRrtB5pqzUeMOxJ7IABBGab dtbe3ESgCow2HacQhYiQq/aGj/cWsR5lzn6ZhDiGal0/psWn31C0HDENEBEfMWVFaVI5 CF3qiGAgJl0jHyJYlVW42S6kffd/ntD99T3sgIdy8A+U2o7H9AEwIl0WWxeWUONjPjHu 6aJVIt9u2h0Mpbi7Q76UiAa0OjJNssY3NTuUWzO5CviCabNXbM2uotjX4PfhNe1MGsnL mpOw== X-Gm-Message-State: AOAM5322QMonU6Z4ObTy5gXxpLUrpDwO7hPfv03ItGdYegnlg9I/KYJ0 IjLulS5kO2Yz/vr42gvFJGCgNqLm7zxMCg== X-Google-Smtp-Source: ABdhPJwsNRiKlEnU9HaXRTlOHmsjSKc+Q3C3vrXJqYEc2v3Dcw8dT69vQ5pulfpcdeauRktqMV2U5g== X-Received: by 2002:a1c:3546:: with SMTP id c67mr11404914wma.43.1637880750296; Thu, 25 Nov 2021 14:52:30 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:29 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 3/9] run-command API users: use strvec_pushv(), not argv assignment Date: Thu, 25 Nov 2021 23:52:18 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Migrate those run-command API users that assign directly to the "argv" member to use a strvec_pushv() of "args" instead. In these cases it did not make sense to further refactor these callers, e.g. daemon.c could be made to construct the arguments closer to handle(), but that would require moving the construction from its cmd_main() and pass "argv" through two intermediate functions. It would be possible for a change like this to introduce a regression if we were doing: cp.argv = argv; argv[1] = "foo"; And changed the code, as is being done here, to: strvec_pushv(&cp.args, argv); argv[1] = "foo"; But as viewing this change with the "-W" flag reveals none of these functions modify variable that's being pushed afterwards in a way that would introduce such a logic error. Signed-off-by: Ævar Arnfjörð Bjarmason --- add-patch.c | 4 ++-- daemon.c | 2 +- http-backend.c | 2 +- http.c | 5 +++-- remote-curl.c | 2 +- run-command.c | 2 +- t/helper/test-subprocess.c | 2 +- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/add-patch.c b/add-patch.c index 8c41cdfe39b..573eef0cc4a 100644 --- a/add-patch.c +++ b/add-patch.c @@ -413,7 +413,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) strvec_push(&args, ps->items[i].original); setup_child_process(s, &cp, NULL); - cp.argv = args.v; + strvec_pushv(&cp.args, args.v); res = capture_command(&cp, plain, 0); if (res) { strvec_clear(&args); @@ -431,7 +431,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) setup_child_process(s, &colored_cp, NULL); xsnprintf((char *)args.v[color_arg_index], 8, "--color"); - colored_cp.argv = args.v; + strvec_pushv(&colored_cp.args, args.v); colored = &s->colored; res = capture_command(&colored_cp, colored, 0); strvec_clear(&args); diff --git a/daemon.c b/daemon.c index b1fcbe0d6fa..8df21f2130c 100644 --- a/daemon.c +++ b/daemon.c @@ -922,7 +922,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) #endif } - cld.argv = cld_argv.v; + strvec_pushv(&cld.args, cld_argv.v); cld.in = incoming; cld.out = dup(incoming); diff --git a/http-backend.c b/http-backend.c index 3d6e2ff17f8..4dd4d939f8a 100644 --- a/http-backend.c +++ b/http-backend.c @@ -480,7 +480,7 @@ static void run_service(const char **argv, int buffer_input) strvec_pushf(&cld.env_array, "GIT_COMMITTER_EMAIL=%s@http.%s", user, host); - cld.argv = argv; + strvec_pushv(&cld.args, argv); if (buffer_input || gzipped_request || req_len >= 0) cld.in = -1; cld.git_cmd = 1; diff --git a/http.c b/http.c index f92859f43fa..229da4d1488 100644 --- a/http.c +++ b/http.c @@ -2126,8 +2126,9 @@ int finish_http_pack_request(struct http_pack_request *preq) ip.git_cmd = 1; ip.in = tmpfile_fd; - ip.argv = preq->index_pack_args ? preq->index_pack_args - : default_index_pack_args; + strvec_pushv(&ip.args, preq->index_pack_args ? + preq->index_pack_args : + default_index_pack_args); if (preq->preserve_index_pack_stdout) ip.out = 0; diff --git a/remote-curl.c b/remote-curl.c index d69156312bd..0dabef2dd7c 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -1061,7 +1061,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, client.in = -1; client.out = -1; client.git_cmd = 1; - client.argv = client_argv; + strvec_pushv(&client.args, client_argv); if (start_command(&client)) exit(1); write_or_die(client.in, preamble->buf, preamble->len); diff --git a/run-command.c b/run-command.c index f40df01c772..620a06ca2f5 100644 --- a/run-command.c +++ b/run-command.c @@ -1039,7 +1039,7 @@ int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir, const char *const *env, const char *tr2_class) { struct child_process cmd = CHILD_PROCESS_INIT; - cmd.argv = argv; + strvec_pushv(&cmd.args, argv); cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0; cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0; diff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c index 92b69de6352..ff22f2fa2c5 100644 --- a/t/helper/test-subprocess.c +++ b/t/helper/test-subprocess.c @@ -15,6 +15,6 @@ int cmd__subprocess(int argc, const char **argv) argv++; } cp.git_cmd = 1; - cp.argv = (const char **)argv + 1; + strvec_pushv(&cp.args, (const char **)argv + 1); return run_command(&cp); } From patchwork Thu Nov 25 22:52:19 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: 12639941 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A06E0C433FE for ; Thu, 25 Nov 2021 22:54:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243827AbhKYW5u (ORCPT ); Thu, 25 Nov 2021 17:57:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57572 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236157AbhKYWzo (ORCPT ); Thu, 25 Nov 2021 17:55:44 -0500 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C8F40C06174A for ; Thu, 25 Nov 2021 14:52:32 -0800 (PST) Received: by mail-wm1-x32a.google.com with SMTP id i8-20020a7bc948000000b0030db7b70b6bso9407374wml.1 for ; Thu, 25 Nov 2021 14:52:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=eQ4PjCDnLFArC5f08daji1CSyq+B7ba+7v/SclBvCuA=; b=EPSiCpYnHV/U7TjUINXY/KsZ0g1RD/CGJKOhrgKa82jrn9BNAKICw21v/5jnMRfnx7 XkiGn2e22sGo3R/XIUnOE+ZUvgwaDi1eKCGAVQJ6O+W6g0Rfl//OVyYYwhgusg6BQFc2 sLp7tS7J5Da+HLI/ZTkIX8EGNGrZH4HUxniVTEXnKgvlusJJMKokRUt3JE+ttW5owIqA mAMl4QaHo45jSpTZC9JAGDnWFGK7Na5cMc9sutWbb09X63RLyNwTwBEqCThsVu47fdUc yJNyj+CTdl+dc6fVuiRlRBr6sMJLSJuJ7AJhgfJEWHrTa3tyoch3Udw9BXYUTibTdWe7 k9Bw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=eQ4PjCDnLFArC5f08daji1CSyq+B7ba+7v/SclBvCuA=; b=5aH6co5SMsUC3K9lkTA952vKXGEhrPsbuZeRMPbbKGKffzM4hVtrX5K2UJBJ9attUx 1WPZsxNEntuzbMzI40RFL9UIxle2sDnXvFCY+Bsvdt/r4P1C9NCu0Y7sjXjf0JZ22ol4 hsZr/sF5cOO0F7+TzkmQkld5CboIwjtcyS8RmXGj7sqK87nejLNGrMbvqmq5CctdmX0J VP5bEk34iJeAjWRrkUf/lhe5LKNI93s11yRirRwN/gpmQDhg3Ql6bk2LrA21iTO6uMFd lcaqPX47nETcyQOeqoSB1a3bpI8m46R3YvFwpy6DdagTPTk0rcdf4FsZ7pzebCzNFRru 0ehQ== X-Gm-Message-State: AOAM532lVeV654+yNug7p2zZC0GH97/Ai+7VbLEMzkDPAs0zHpWkjfuC gXcLhZA5ds/Q96uedXMFLJVH34rBHaqiuQ== X-Google-Smtp-Source: ABdhPJyBa5v1wdUUh9CgFfeEkXAPRwM80qLfgWYHbCVBOf9ZcsHUjXB84jTCbazkAz9fGznV1KBfSA== X-Received: by 2002:a05:600c:4e45:: with SMTP id e5mr11820009wmq.43.1637880751147; Thu, 25 Nov 2021 14:52:31 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:30 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 4/9] run-command tests: use strvec_pushv(), not argv assignment Date: Thu, 25 Nov 2021 23:52:19 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As in the preceding commit change this API user to use strvec_pushv() instead of assigning to the "argv" member directly. This leaves us without test coverage of how the "argv" assignment in this API works, but we'll be removing it in a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/helper/test-run-command.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 3c4fb862234..913775a14b7 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -31,7 +31,7 @@ static int parallel_next(struct child_process *cp, if (number_callbacks >= 4) return 0; - strvec_pushv(&cp->args, d->argv); + strvec_pushv(&cp->args, d->args.v); strbuf_addstr(err, "preloaded output of a child\n"); number_callbacks++; return 1; @@ -274,7 +274,7 @@ static int quote_stress_test(int argc, const char **argv) if (i < skip) continue; - cp.argv = args.v; + strvec_pushv(&cp.args, args.v); strbuf_reset(&out); if (pipe_command(&cp, NULL, 0, &out, 0, NULL, 0) < 0) return error("Failed to spawn child process"); @@ -396,7 +396,7 @@ int cmd__run_command(int argc, const char **argv) } if (argc < 3) return 1; - proc.argv = (const char **)argv + 2; + strvec_pushv(&proc.args, (const char **)argv + 2); if (!strcmp(argv[1], "start-command-ENOENT")) { if (start_command(&proc) < 0 && errno == ENOENT) @@ -408,7 +408,8 @@ int cmd__run_command(int argc, const char **argv) exit(run_command(&proc)); jobs = atoi(argv[2]); - proc.argv = (const char **)argv + 3; + strvec_clear(&proc.args); + strvec_pushv(&proc.args, (const char **)argv + 3); if (!strcmp(argv[1], "run-command-parallel")) exit(run_processes_parallel(jobs, parallel_next, From patchwork Thu Nov 25 22:52:20 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: 12639945 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AFDCC433FE for ; Thu, 25 Nov 2021 22:56:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346150AbhKYW7n (ORCPT ); Thu, 25 Nov 2021 17:59:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232830AbhKYW5l (ORCPT ); Thu, 25 Nov 2021 17:57:41 -0500 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C974FC061756 for ; Thu, 25 Nov 2021 14:52:33 -0800 (PST) Received: by mail-wr1-x435.google.com with SMTP id a9so14604088wrr.8 for ; Thu, 25 Nov 2021 14:52:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=bY+2ZJS1sprpQzhplco7lSIRxZzXqhEHd27hLy0Ykjw=; b=JtTz42Zh64gvOWOoKQUQlFuAA3h0XqY64d1YF4m0Z6xu62Uz07QUt7pC4Sv4duVqnS VKJibVLyOT89QHXuj4RWkg7cS9iSmOaBmUlRcC9RofhR1Lma99TXrXx28DN5rqHj3SWA 7FqB3koQJR5XRKrGvl8YHIJkAYNZ8SkIXjiaJcePTlIDkOYeIKhvWq+XpGxM70sGHjBm ytyg5NJ/XbEGj4tEWOdmSYpue/ehHlnpQy1qVRz6gV3dqjbi+COIQ9GZFObMyZ1suO04 qrBf6ak2YRJgS8vDP9zW0GU5GAdRcdRgKSk5qKWBeqpbrLgTkKwWo9xutdNz9tVFmZrY sOnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=bY+2ZJS1sprpQzhplco7lSIRxZzXqhEHd27hLy0Ykjw=; b=eZAlNjcZUkV0pq7iYUBL8wjPjfG+9uhlxR0meoWKszDpvPdUiezTtOT75BLPrGJgck bzK6HovLV/Fwm7qPyS1+s1KobzQ1WcLm0xNPgf5f1jGMVBQtH1g3a+r5JlY5LHbirSrv GXwrikLC8Z0wHmbNp9/WOM9WIaPGxidtrWxOa75otKv31IkWFGA9N2iI6ZZBawswPgln vHuWU4TQQGiLctxeOAcZ/fq+S9z34etY3Ue9aF2iNuv/xsDQmdsHwWIb75kRNP5rLB8g EvGLMXvBKgYq35wR2zXGiVkhFOnfptEjwIV4FJ4j+IrvTQ38ts6e6hQ7K0YFZWa3cdpe 2JDg== X-Gm-Message-State: AOAM533KWlagnyytFCX2FlCQOx/oFJeaL3pvXM3p5/IPtqw++FCNN7Gv TEI5V4xudbcn+7ORhhHhivPMsw/cGM1HPQ== X-Google-Smtp-Source: ABdhPJyyvToLjrOTiuVp9Zgm9mx9mS2GC/JXU/Za2Mgn8C95Iht4dTtPYY0cORugQ8YtI7oKZwJ4+A== X-Received: by 2002:a5d:4e0b:: with SMTP id p11mr10437961wrt.88.1637880752108; Thu, 25 Nov 2021 14:52:32 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:31 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 5/9] run-command API users: use strvec_pushl(), not argv construction Date: Thu, 25 Nov 2021 23:52:20 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_pushl()" to add data to the "args" member. This implements the same behavior as before in fewer lines of code, and moves us further towards being able to remove the "argv" member in a subsequent commit. Since we've entirely removed the "argv" variable(s) we can be sure that no potential logic errors of the type discussed in a preceding commit are being introduced here, i.e. ones where the local "argv" was being modified after the assignment to "struct child_process"'s "argv". Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/add.c | 7 ++----- builtin/fsck.c | 12 ++++-------- builtin/help.c | 3 +-- builtin/merge.c | 3 +-- builtin/notes.c | 5 ++--- builtin/receive-pack.c | 38 +++++++++++++------------------------- builtin/replace.c | 3 +-- editor.c | 4 +--- sequencer.c | 10 +++------- upload-pack.c | 5 +---- 10 files changed, 29 insertions(+), 61 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index ef6b619c45e..a010b2c325f 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -302,15 +302,11 @@ int interactive_add(const char **argv, const char *prefix, int patch) static int edit_patch(int argc, const char **argv, const char *prefix) { char *file = git_pathdup("ADD_EDIT.patch"); - const char *apply_argv[] = { "apply", "--recount", "--cached", - NULL, NULL }; struct child_process child = CHILD_PROCESS_INIT; struct rev_info rev; int out; struct stat st; - apply_argv[3] = file; - git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ if (read_cache() < 0) @@ -338,7 +334,8 @@ static int edit_patch(int argc, const char **argv, const char *prefix) die(_("Empty patch. Aborted.")); child.git_cmd = 1; - child.argv = apply_argv; + strvec_pushl(&child.args, "apply", "--recount", "--cached", file, + NULL); if (run_command(&child)) die(_("Could not apply '%s'"), file); diff --git a/builtin/fsck.c b/builtin/fsck.c index 27b9e78094d..9e54892311d 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -944,15 +944,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) if (the_repository->settings.core_commit_graph) { struct child_process commit_graph_verify = CHILD_PROCESS_INIT; - const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL }; prepare_alt_odb(the_repository); for (odb = the_repository->objects->odb; odb; odb = odb->next) { child_process_init(&commit_graph_verify); - commit_graph_verify.argv = verify_argv; commit_graph_verify.git_cmd = 1; - verify_argv[2] = "--object-dir"; - verify_argv[3] = odb->path; + strvec_pushl(&commit_graph_verify.args, "commit-graph", + "verify", "--object-dir", odb->path, NULL); if (run_command(&commit_graph_verify)) errors_found |= ERROR_COMMIT_GRAPH; } @@ -960,15 +958,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) if (the_repository->settings.core_multi_pack_index) { struct child_process midx_verify = CHILD_PROCESS_INIT; - const char *midx_argv[] = { "multi-pack-index", "verify", NULL, NULL, NULL }; prepare_alt_odb(the_repository); for (odb = the_repository->objects->odb; odb; odb = odb->next) { child_process_init(&midx_verify); - midx_verify.argv = midx_argv; midx_verify.git_cmd = 1; - midx_argv[2] = "--object-dir"; - midx_argv[3] = odb->path; + strvec_pushl(&midx_verify.args, "multi-pack-index", + "verify", "--object-dir", odb->path, NULL); if (run_command(&midx_verify)) errors_found |= ERROR_MULTI_PACK_INDEX; } diff --git a/builtin/help.c b/builtin/help.c index 75cd2fb407f..d387131dd83 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -212,11 +212,10 @@ static int check_emacsclient_version(void) { struct strbuf buffer = STRBUF_INIT; struct child_process ec_process = CHILD_PROCESS_INIT; - const char *argv_ec[] = { "emacsclient", "--version", NULL }; int version; /* emacsclient prints its version number on stderr */ - ec_process.argv = argv_ec; + strvec_pushl(&ec_process.args, "emacsclient", "--version", NULL); ec_process.err = -1; ec_process.stdout_to_stderr = 1; if (start_command(&ec_process)) diff --git a/builtin/merge.c b/builtin/merge.c index ea3112e0c0b..5f0476b0b76 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -310,10 +310,9 @@ static int save_state(struct object_id *stash) int len; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf buffer = STRBUF_INIT; - const char *argv[] = {"stash", "create", NULL}; int rc = -1; - cp.argv = argv; + strvec_pushl(&cp.args, "stash", "create", NULL); cp.out = -1; cp.git_cmd = 1; diff --git a/builtin/notes.c b/builtin/notes.c index 71c59583a17..85d1abad884 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -134,14 +134,13 @@ static void copy_obj_to_fd(int fd, const struct object_id *oid) static void write_commented_object(int fd, const struct object_id *object) { - const char *show_args[5] = - {"show", "--stat", "--no-notes", oid_to_hex(object), NULL}; struct child_process show = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; struct strbuf cbuf = STRBUF_INIT; /* Invoke "git show --stat --no-notes $object" */ - show.argv = show_args; + strvec_pushl(&show.args, "show", "--stat", "--no-notes", + oid_to_hex(object), NULL); show.no_stdin = 1; show.out = -1; show.err = 0; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 49b846d9605..6149d507965 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1370,22 +1370,10 @@ static const char *push_to_deploy(unsigned char *sha1, struct strvec *env, const char *work_tree) { - const char *update_refresh[] = { - "update-index", "-q", "--ignore-submodules", "--refresh", NULL - }; - const char *diff_files[] = { - "diff-files", "--quiet", "--ignore-submodules", "--", NULL - }; - const char *diff_index[] = { - "diff-index", "--quiet", "--cached", "--ignore-submodules", - NULL, "--", NULL - }; - const char *read_tree[] = { - "read-tree", "-u", "-m", NULL, NULL - }; struct child_process child = CHILD_PROCESS_INIT; - child.argv = update_refresh; + strvec_pushl(&child.args, "update-index", "-q", "--ignore-submodules", + "--refresh", NULL); child.env = env->v; child.dir = work_tree; child.no_stdin = 1; @@ -1396,7 +1384,8 @@ static const char *push_to_deploy(unsigned char *sha1, /* run_command() does not clean up completely; reinitialize */ child_process_init(&child); - child.argv = diff_files; + strvec_pushl(&child.args, "diff-files", "--quiet", + "--ignore-submodules", "--", NULL); child.env = env->v; child.dir = work_tree; child.no_stdin = 1; @@ -1405,11 +1394,12 @@ static const char *push_to_deploy(unsigned char *sha1, if (run_command(&child)) return "Working directory has unstaged changes"; - /* diff-index with either HEAD or an empty tree */ - diff_index[4] = head_has_history() ? "HEAD" : empty_tree_oid_hex(); - child_process_init(&child); - child.argv = diff_index; + strvec_pushl(&child.args, "diff-index", "--quiet", "--cached", + "--ignore-submodules", + /* diff-index with either HEAD or an empty tree */ + head_has_history() ? "HEAD" : empty_tree_oid_hex(), + "--", NULL); child.env = env->v; child.no_stdin = 1; child.no_stdout = 1; @@ -1418,9 +1408,9 @@ static const char *push_to_deploy(unsigned char *sha1, if (run_command(&child)) return "Working directory has staged changes"; - read_tree[3] = hash_to_hex(sha1); child_process_init(&child); - child.argv = read_tree; + strvec_pushl(&child.args, "read-tree", "-u", "-m", hash_to_hex(sha1), + NULL); child.env = env->v; child.dir = work_tree; child.no_stdin = 1; @@ -2575,16 +2565,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) run_update_post_hook(commands); string_list_clear(&push_options, 0); if (auto_gc) { - const char *argv_gc_auto[] = { - "gc", "--auto", "--quiet", NULL, - }; struct child_process proc = CHILD_PROCESS_INIT; proc.no_stdin = 1; proc.stdout_to_stderr = 1; proc.err = use_sideband ? -1 : 0; proc.git_cmd = proc.close_object_store = 1; - proc.argv = argv_gc_auto; + strvec_pushl(&proc.args, "gc", "--auto", "--quiet", + NULL); if (!start_command(&proc)) { if (use_sideband) diff --git a/builtin/replace.c b/builtin/replace.c index 946938d011e..6ff1734d587 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -258,11 +258,10 @@ static int import_object(struct object_id *oid, enum object_type type, return error_errno(_("unable to open %s for reading"), filename); if (!raw && type == OBJ_TREE) { - const char *argv[] = { "mktree", NULL }; struct child_process cmd = CHILD_PROCESS_INIT; struct strbuf result = STRBUF_INIT; - cmd.argv = argv; + strvec_push(&cmd.args, "mktree"); cmd.git_cmd = 1; cmd.in = fd; cmd.out = -1; diff --git a/editor.c b/editor.c index fdd3eeafa94..d92a8d9ab5b 100644 --- a/editor.c +++ b/editor.c @@ -55,7 +55,6 @@ static int launch_specified_editor(const char *editor, const char *path, if (strcmp(editor, ":")) { struct strbuf realpath = STRBUF_INIT; - const char *args[] = { editor, NULL, NULL }; struct child_process p = CHILD_PROCESS_INIT; int ret, sig; int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) && isatty(2); @@ -77,9 +76,8 @@ static int launch_specified_editor(const char *editor, const char *path, } strbuf_realpath(&realpath, path, 1); - args[1] = realpath.buf; - p.argv = args; + strvec_pushl(&p.args, editor, realpath.buf, NULL); p.env = env; p.use_shell = 1; p.trace2_child_class = "editor"; diff --git a/sequencer.c b/sequencer.c index ea96837cde3..6e02210db7a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1164,18 +1164,14 @@ static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { struct child_process proc = CHILD_PROCESS_INIT; - const char *argv[3]; int code; struct strbuf sb = STRBUF_INIT; + const char *hook_path = find_hook("post-rewrite"); - argv[0] = find_hook("post-rewrite"); - if (!argv[0]) + if (!hook_path) return 0; - argv[1] = "amend"; - argv[2] = NULL; - - proc.argv = argv; + strvec_pushl(&proc.args, hook_path, "amend", NULL); proc.in = -1; proc.stdout_to_stderr = 1; proc.trace2_hook_name = "post-rewrite"; diff --git a/upload-pack.c b/upload-pack.c index c78d55bc674..9b5db32623f 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -596,14 +596,11 @@ static int do_reachable_revlist(struct child_process *cmd, struct object_array *reachable, enum allow_uor allow_uor) { - static const char *argv[] = { - "rev-list", "--stdin", NULL, - }; struct object *o; FILE *cmd_in = NULL; int i; - cmd->argv = argv; + strvec_pushl(&cmd->args, "rev-list", "--stdin", NULL); cmd->git_cmd = 1; cmd->no_stderr = 1; cmd->in = -1; From patchwork Thu Nov 25 22:52:21 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: 12639943 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8AFC2C433F5 for ; Thu, 25 Nov 2021 22:56:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346012AbhKYW7m (ORCPT ); Thu, 25 Nov 2021 17:59:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57984 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234127AbhKYW5l (ORCPT ); Thu, 25 Nov 2021 17:57:41 -0500 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0776C061757 for ; Thu, 25 Nov 2021 14:52:34 -0800 (PST) Received: by mail-wm1-x329.google.com with SMTP id 137so6716732wma.1 for ; Thu, 25 Nov 2021 14:52:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=OsBcJli4MkbJ8Y8iguiFgc2zzaV6UDo8tky+jVmrIcM=; b=Mz6ExfPnOBYQlO9NLvTjfG3Kl4wErcqnzIWVSwRVFmjx3qYF1VK9kpPZajQsuCS9vW arhiUKo5KAXoJxjeYaUYziwHEZf532siISRM3ZGDnFZCrTsIwR3M2SSDo7m3dwSvurZz z65TjZlRgYVjcOmCBNqo226UwUiJnaZdA8lukQP183Uangm5b04RcB93vYhV8wpT1y08 7jvzziotbsLKJZ1KKLE/BTrqC81Olm7lKWGE3Krlze1LR1itceaFhzr87GAySmy8fD29 HB7EdSjcTL1G/7xWIdR+UgyBA2F19yQW2A6J01fms2KYEYk7Cej/GpcuBqJzbkV5Pdae aVWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OsBcJli4MkbJ8Y8iguiFgc2zzaV6UDo8tky+jVmrIcM=; b=FGGUpRjsdwUIEFPR/OYXqUMvmCQ21EjroT4pA4kh9Yrq+ib9RaP2nOPBEnSwCOzRJL 994hz0ObtJcU/NO7CI5YB03BmBLsh6vwpg4LUo6Bm23Ig5opqsVOcilU5meE8UWPMuSc jbY0a4WfPF0gTxKuFSejnDqL5KFTLzfQ9j4pGVw4yVFj+xF/tQqzB8+Xq9u6mcvCDhWU WhUbKTRklOgDTNMW144VrQVuLWEMeHOPJKqdWwucK5/CDloOxjPv3u2TgGXJhSQ3iVm8 0rEtA4UOwu4npJeqUqFmw6EVvNGjEm+cNBVU5aZSES4B9yLCNalb/QaDTx4PN7E3bqW2 RxkA== X-Gm-Message-State: AOAM533lxBF4PQKI4wuUlcz68BhjhkPWYQsSv772aaYVSanEkfVYYnzC OFO/ciQuPvzcLHEExChLsw2x+ZcBP0hzhA== X-Google-Smtp-Source: ABdhPJxCI8/PyWBNIW26flOfja+eOlxRbBL1VuI7htxFD3DQdmsr/S6O5bYsYf9p4QAyj/n6x3Vi1w== X-Received: by 2002:a7b:c341:: with SMTP id l1mr12045761wmj.60.1637880753215; Thu, 25 Nov 2021 14:52:33 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:32 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 6/9] run-command API users: use strvec_push(), not argv construction Date: Thu, 25 Nov 2021 23:52:21 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_push()" to add data to the "args" member. As noted in the preceding commit this moves us further towards being able to remove the "argv" member in a subsequent commit These callers could have used strvec_pushl(), but moving to strvec_push() makes the diff easier to read, and keeps the arguments aligned as before. Signed-off-by: Ævar Arnfjörð Bjarmason --- archive-tar.c | 9 +++------ builtin/receive-pack.c | 31 ++++++++++++------------------- daemon.c | 18 +++++++----------- diff.c | 8 ++------ prompt.c | 7 ++----- transport.c | 11 +++++------ 6 files changed, 31 insertions(+), 53 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 05d2455870d..3c74db17468 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -430,7 +430,6 @@ static int write_tar_filter_archive(const struct archiver *ar, { struct strbuf cmd = STRBUF_INIT; struct child_process filter = CHILD_PROCESS_INIT; - const char *argv[2]; int r; if (!ar->data) @@ -440,14 +439,12 @@ static int write_tar_filter_archive(const struct archiver *ar, if (args->compression_level >= 0) strbuf_addf(&cmd, " -%d", args->compression_level); - argv[0] = cmd.buf; - argv[1] = NULL; - filter.argv = argv; + strvec_push(&filter.args, cmd.buf); filter.use_shell = 1; filter.in = -1; if (start_command(&filter) < 0) - die_errno(_("unable to start '%s' filter"), argv[0]); + die_errno(_("unable to start '%s' filter"), cmd.buf); close(1); if (dup2(filter.in, 1) < 0) die_errno(_("unable to redirect descriptor")); @@ -457,7 +454,7 @@ static int write_tar_filter_archive(const struct archiver *ar, close(1); if (finish_command(&filter) != 0) - die(_("'%s' filter reported error"), argv[0]); + die(_("'%s' filter reported error"), cmd.buf); strbuf_release(&cmd); return r; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 6149d507965..48c99c8ee45 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -812,16 +812,13 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, { struct child_process proc = CHILD_PROCESS_INIT; struct async muxer; - const char *argv[2]; int code; + const char *hook_path = find_hook(hook_name); - argv[0] = find_hook(hook_name); - if (!argv[0]) + if (!hook_path) return 0; - argv[1] = NULL; - - proc.argv = argv; + strvec_push(&proc.args, hook_path); proc.in = -1; proc.stdout_to_stderr = 1; proc.trace2_hook_name = hook_name; @@ -943,23 +940,21 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - const char *argv[5]; struct child_process proc = CHILD_PROCESS_INIT; int code; + const char *hook_path = find_hook("update"); - argv[0] = find_hook("update"); - if (!argv[0]) + if (!hook_path) return 0; - argv[1] = cmd->ref_name; - argv[2] = oid_to_hex(&cmd->old_oid); - argv[3] = oid_to_hex(&cmd->new_oid); - argv[4] = NULL; + strvec_push(&proc.args, hook_path); + strvec_push(&proc.args, cmd->ref_name); + strvec_push(&proc.args, oid_to_hex(&cmd->old_oid)); + strvec_push(&proc.args, oid_to_hex(&cmd->new_oid)); proc.no_stdin = 1; proc.stdout_to_stderr = 1; proc.err = use_sideband ? -1 : 0; - proc.argv = argv; proc.trace2_hook_name = "update"; code = start_command(&proc); @@ -1117,22 +1112,20 @@ static int run_proc_receive_hook(struct command *commands, struct child_process proc = CHILD_PROCESS_INIT; struct async muxer; struct command *cmd; - const char *argv[2]; struct packet_reader reader; struct strbuf cap = STRBUF_INIT; struct strbuf errmsg = STRBUF_INIT; int hook_use_push_options = 0; int version = 0; int code; + const char *hook_path = find_hook("proc-receive"); - argv[0] = find_hook("proc-receive"); - if (!argv[0]) { + if (!hook_path) { rp_error("cannot find hook 'proc-receive'"); return -1; } - argv[1] = NULL; - proc.argv = argv; + strvec_push(&proc.args, hook_path); proc.in = -1; proc.out = -1; proc.trace2_hook_name = "proc-receive"; diff --git a/daemon.c b/daemon.c index 8df21f2130c..4a000ee4afa 100644 --- a/daemon.c +++ b/daemon.c @@ -326,22 +326,18 @@ static int run_access_hook(struct daemon_service *service, const char *dir, { struct child_process child = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; - const char *argv[8]; - const char **arg = argv; char *eol; int seen_errors = 0; - *arg++ = access_hook; - *arg++ = service->name; - *arg++ = path; - *arg++ = hi->hostname.buf; - *arg++ = get_canon_hostname(hi); - *arg++ = get_ip_address(hi); - *arg++ = hi->tcp_port.buf; - *arg = NULL; + strvec_push(&child.args, access_hook); + strvec_push(&child.args, service->name); + strvec_push(&child.args, path); + strvec_push(&child.args, hi->hostname.buf); + strvec_push(&child.args, get_canon_hostname(hi)); + strvec_push(&child.args, get_ip_address(hi)); + strvec_push(&child.args, hi->tcp_port.buf); child.use_shell = 1; - child.argv = argv; child.no_stdin = 1; child.no_stderr = 1; child.out = -1; diff --git a/diff.c b/diff.c index 861282db1c3..41076857428 100644 --- a/diff.c +++ b/diff.c @@ -6921,19 +6921,15 @@ static char *run_textconv(struct repository *r, size_t *outsize) { struct diff_tempfile *temp; - const char *argv[3]; - const char **arg = argv; struct child_process child = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; int err = 0; temp = prepare_temp_file(r, spec->path, spec); - *arg++ = pgm; - *arg++ = temp->name; - *arg = NULL; + strvec_push(&child.args, pgm); + strvec_push(&child.args, temp->name); child.use_shell = 1; - child.argv = argv; child.out = -1; if (start_command(&child)) { remove_tempfile(); diff --git a/prompt.c b/prompt.c index 5ded21a017f..50df17279d1 100644 --- a/prompt.c +++ b/prompt.c @@ -8,15 +8,12 @@ static char *do_askpass(const char *cmd, const char *prompt) { struct child_process pass = CHILD_PROCESS_INIT; - const char *args[3]; static struct strbuf buffer = STRBUF_INIT; int err = 0; - args[0] = cmd; - args[1] = prompt; - args[2] = NULL; + strvec_push(&pass.args, cmd); + strvec_push(&pass.args, prompt); - pass.argv = args; pass.out = -1; if (start_command(&pass)) diff --git a/transport.c b/transport.c index e4f1decae20..92ab9a3fa6b 100644 --- a/transport.c +++ b/transport.c @@ -1204,16 +1204,15 @@ static int run_pre_push_hook(struct transport *transport, struct ref *r; struct child_process proc = CHILD_PROCESS_INIT; struct strbuf buf; - const char *argv[4]; + const char *hook_path = find_hook("pre-push"); - if (!(argv[0] = find_hook("pre-push"))) + if (!hook_path) return 0; - argv[1] = transport->remote->name; - argv[2] = transport->url; - argv[3] = NULL; + strvec_push(&proc.args, hook_path); + strvec_push(&proc.args, transport->remote->name); + strvec_push(&proc.args, transport->url); - proc.argv = argv; proc.in = -1; proc.trace2_hook_name = "pre-push"; From patchwork Thu Nov 25 22:52:22 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: 12639947 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AFB62C433F5 for ; Thu, 25 Nov 2021 22:56:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346263AbhKYW7q (ORCPT ); Thu, 25 Nov 2021 17:59:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57986 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230091AbhKYW5m (ORCPT ); Thu, 25 Nov 2021 17:57:42 -0500 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 EADC1C061758 for ; Thu, 25 Nov 2021 14:52:35 -0800 (PST) Received: by mail-wr1-x42a.google.com with SMTP id d24so14688027wra.0 for ; Thu, 25 Nov 2021 14:52:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ktV1uXEGvzIUIz+w+85KX4GkKFIlhKJkGJ/a1K0n4DM=; b=GUHZqluabodUEZ9BzbMgWpqwXjopc5u9dyc7MzH3EOSWl0QaUHtFUMHw7qr8Q+A8eS AiO6DOd4QFqkaGAde3Q0brXMch8jd/Yyg25ZFaXO0K9jfxaRRW8OeUXCdT5n4KuOnyoI TBuCaM1x3Aq+lfxDbEk+3x0/gXJvu52SLzmQkRc4TdKTgeGwf6ODyBkbCfvhkw6ivYuk CE55uXIQwzQRFsVwbjUtswMXWj/F9aOkWCoIOGzsVg6wGgk+EJFNvgjkE8tkRDQpDCqE smdeVVWc8LWXjAP2FriaeYUypjykMtN5fx3xEbaOemsrB/sbwyoTUM+HJlsHG/m49ims huHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ktV1uXEGvzIUIz+w+85KX4GkKFIlhKJkGJ/a1K0n4DM=; b=nfmi+yvOWmEuDDWJFOj3KPOn+YUTDMs+ZYG+PwSkMqz9ByItajkupDVI41PhV6C89h v2j5hq7y1Ir0OXIeOk1z94+xii9D9PqYVsTb9gtVAvNLx9oI7Bituklw4/IuZvqUkY4I VIWxYEXNplTlh+Y3puZ7t3TjBIserOpM9qzewBGn3YjVyq9KAw64Q/i2kNlNchH+u0lk 4uvsd6MphTooJbiJj+t0PiilAIiquxrbcYEnKAvoweXud/DtZk8gnZBwSDdYW0+APcDz y1ZgqHQeE386X3B1Kha/EfmNRkKbV4iZyQMJjDerdMsOqO0tZH/mjemS7/vS4YtCR7Mq ByWQ== X-Gm-Message-State: AOAM530ZjjC2RprNa1lplP2NpJvQ1CNm30nPJI7sfoVFtGp1kkMEqI8F MlDUWfSKh6UsBXoNRyVcbMc4TSf15SAKPA== X-Google-Smtp-Source: ABdhPJzO4fsprqrEySNhXIBhA4bw7IGUTpz9Jqd9x+UMub76RTLlyBhLywlr2I1eFlkmwmyid4z6BA== X-Received: by 2002:a05:6000:1a41:: with SMTP id t1mr10552018wry.261.1637880754250; Thu, 25 Nov 2021 14:52:34 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:33 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 7/9] run-command API: remove "argv" member, always use "args" Date: Thu, 25 Nov 2021 23:52:22 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Remove the "argv" member from the run-command API, ever since "args" was added in c460c0ecdca (run-command: store an optional argv_array, 2014-05-15) being able to provide either "argv" or "args" has led to some confusion and bugs. If we hadn't gone in that direction and only had an "argv" our problems wouldn't have been solved either, as noted in [1] (and in the documentation amended here) it comes with inherent memory management issues: The caller would have to hang on to the "argv" until the run-command API was finished. If the "argv" was an argument to main() this wasn't an issue, but if it it was manually constructed using the API might be painful. We also have a recent report[2] of a user of the API segfaulting, which is a direct result of it being complex to use. This commit addresses the root cause of that bug. This change is larger than I'd like, but there's no easy way to avoid it that wouldn't involve even more verbose intermediate steps. We use the "argv" as the source of truth over the "args", so we need to change all parts of run-command.[ch] itself, as well as the trace2 logging at the same time. The resulting Windows-specific code in start_command() is a bit nasty, as we're now assigning to a strvec's "v" member, instead of to our own "argv". There was a suggestion of some alternate approaches in reply to an earlier version of this commit[3], but let's leave larger a larger and needless refactoring of this code for now. 1. http://lore.kernel.org/git/YT6BnnXeAWn8BycF@coredump.intra.peff.net 2. https://lore.kernel.org/git/20211120194048.12125-1-ematsumiya@suse.de/ 3. https://lore.kernel.org/git/patch-5.5-ea1011f7473-20211122T153605Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason --- run-command.c | 42 ++++++++++++++++++++--------------------- run-command.h | 20 ++++++++------------ sub-process.c | 2 +- trace2/tr2_tgt_event.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- trace2/tr2_tgt_perf.c | 4 ++-- 6 files changed, 33 insertions(+), 39 deletions(-) diff --git a/run-command.c b/run-command.c index 620a06ca2f5..99dc93e7300 100644 --- a/run-command.c +++ b/run-command.c @@ -380,7 +380,7 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr) switch (cerr->err) { case CHILD_ERR_CHDIR: error_errno("exec '%s': cd to '%s' failed", - cmd->argv[0], cmd->dir); + cmd->args.v[0], cmd->dir); break; case CHILD_ERR_DUP2: error_errno("dup2() in child failed"); @@ -392,12 +392,12 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr) error_errno("sigprocmask failed restoring signals"); break; case CHILD_ERR_ENOENT: - error_errno("cannot run %s", cmd->argv[0]); + error_errno("cannot run %s", cmd->args.v[0]); break; case CHILD_ERR_SILENT: break; case CHILD_ERR_ERRNO: - error_errno("cannot exec '%s'", cmd->argv[0]); + error_errno("cannot exec '%s'", cmd->args.v[0]); break; } set_error_routine(old_errfn); @@ -405,7 +405,7 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr) static int prepare_cmd(struct strvec *out, const struct child_process *cmd) { - if (!cmd->argv[0]) + if (!cmd->args.v[0]) BUG("command is empty"); /* @@ -415,11 +415,11 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd) strvec_push(out, SHELL_PATH); if (cmd->git_cmd) { - prepare_git_cmd(out, cmd->argv); + prepare_git_cmd(out, cmd->args.v); } else if (cmd->use_shell) { - prepare_shell_cmd(out, cmd->argv); + prepare_shell_cmd(out, cmd->args.v); } else { - strvec_pushv(out, cmd->argv); + strvec_pushv(out, cmd->args.v); } /* @@ -663,7 +663,7 @@ static void trace_run_command(const struct child_process *cp) trace_add_env(&buf, cp->env); if (cp->git_cmd) strbuf_addstr(&buf, " git"); - sq_quote_argv_pretty(&buf, cp->argv); + sq_quote_argv_pretty(&buf, cp->args.v); trace_printf("%s", buf.buf); strbuf_release(&buf); @@ -676,8 +676,6 @@ int start_command(struct child_process *cmd) int failed_errno; char *str; - if (!cmd->argv) - cmd->argv = cmd->args.v; if (!cmd->env) cmd->env = cmd->env_array.v; @@ -729,7 +727,7 @@ int start_command(struct child_process *cmd) str = "standard error"; fail_pipe: error("cannot create %s pipe for %s: %s", - str, cmd->argv[0], strerror(failed_errno)); + str, cmd->args.v[0], strerror(failed_errno)); child_process_clear(cmd); errno = failed_errno; return -1; @@ -758,7 +756,7 @@ int start_command(struct child_process *cmd) failed_errno = errno; cmd->pid = -1; if (!cmd->silent_exec_failure) - error_errno("cannot run %s", cmd->argv[0]); + error_errno("cannot run %s", cmd->args.v[0]); goto end_of_spawn; } @@ -868,7 +866,7 @@ int start_command(struct child_process *cmd) } atfork_parent(&as); if (cmd->pid < 0) - error_errno("cannot fork() for %s", cmd->argv[0]); + error_errno("cannot fork() for %s", cmd->args.v[0]); else if (cmd->clean_on_exit) mark_child_for_cleanup(cmd->pid, cmd); @@ -885,7 +883,7 @@ int start_command(struct child_process *cmd) * At this point we know that fork() succeeded, but exec() * failed. Errors have been reported to our stderr. */ - wait_or_whine(cmd->pid, cmd->argv[0], 0); + wait_or_whine(cmd->pid, cmd->args.v[0], 0); child_err_spew(cmd, &cerr); failed_errno = errno; cmd->pid = -1; @@ -902,7 +900,7 @@ int start_command(struct child_process *cmd) #else { int fhin = 0, fhout = 1, fherr = 2; - const char **sargv = cmd->argv; + const char **sargv = cmd->args.v; struct strvec nargv = STRVEC_INIT; if (cmd->no_stdin) @@ -929,20 +927,20 @@ int start_command(struct child_process *cmd) fhout = dup(cmd->out); if (cmd->git_cmd) - cmd->argv = prepare_git_cmd(&nargv, cmd->argv); + cmd->args.v = prepare_git_cmd(&nargv, sargv); else if (cmd->use_shell) - cmd->argv = prepare_shell_cmd(&nargv, cmd->argv); + cmd->args.v = prepare_shell_cmd(&nargv, sargv); - cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env, + cmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v, (char**) cmd->env, cmd->dir, fhin, fhout, fherr); failed_errno = errno; if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT)) - error_errno("cannot spawn %s", cmd->argv[0]); + error_errno("cannot spawn %s", cmd->args.v[0]); if (cmd->clean_on_exit && cmd->pid >= 0) mark_child_for_cleanup(cmd->pid, cmd); strvec_clear(&nargv); - cmd->argv = sargv; + cmd->args.v = sargv; if (fhin != 0) close(fhin); if (fhout != 1) @@ -992,7 +990,7 @@ int start_command(struct child_process *cmd) int finish_command(struct child_process *cmd) { - int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0); + int ret = wait_or_whine(cmd->pid, cmd->args.v[0], 0); trace2_child_exit(cmd, ret); child_process_clear(cmd); invalidate_lstat_cache(); @@ -1001,7 +999,7 @@ int finish_command(struct child_process *cmd) int finish_command_in_signal(struct child_process *cmd) { - int ret = wait_or_whine(cmd->pid, cmd->argv[0], 1); + int ret = wait_or_whine(cmd->pid, cmd->args.v[0], 1); trace2_child_exit(cmd, ret); return ret; } diff --git a/run-command.h b/run-command.h index 49878262584..c0d1210cc63 100644 --- a/run-command.h +++ b/run-command.h @@ -44,21 +44,17 @@ struct child_process { /** - * The .argv member is set up as an array of string pointers (NULL - * terminated), of which .argv[0] is the program name to run (usually - * without a path). If the command to run is a git command, set argv[0] to - * the command name without the 'git-' prefix and set .git_cmd = 1. + * The .args is a `struct strvec', use that API to manipulate + * it, e.g. strvec_pushv() to add an existing "const char **" + * vector. * - * Note that the ownership of the memory pointed to by .argv stays with the - * caller, but it should survive until `finish_command` completes. If the - * .argv member is NULL, `start_command` will point it at the .args - * `strvec` (so you may use one or the other, but you must use exactly - * one). The memory in .args will be cleaned up automatically during - * `finish_command` (or during `start_command` when it is unsuccessful). + * If the command to run is a git command, set the first + * element in the strvec to the command name without the + * 'git-' prefix and set .git_cmd = 1. * + * The memory in .args will be cleaned up automatically during + * `finish_command` (or during `start_command` when it is unsuccessful). */ - const char **argv; - struct strvec args; struct strvec env_array; pid_t pid; diff --git a/sub-process.c b/sub-process.c index dfa790d3ff9..cae56ae6b80 100644 --- a/sub-process.c +++ b/sub-process.c @@ -187,7 +187,7 @@ static int handshake_capabilities(struct child_process *process, *supported_capabilities |= capabilities[i].flag; } else { die("subprocess '%s' requested unsupported capability '%s'", - process->argv[0], p); + process->args.v[0], p); } } diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 3a0014417cc..bd17ecdc321 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -354,7 +354,7 @@ static void fn_child_start_fl(const char *file, int line, jw_object_inline_begin_array(&jw, "argv"); if (cmd->git_cmd) jw_array_string(&jw, "git"); - jw_array_argv(&jw, cmd->argv); + jw_array_argv(&jw, cmd->args.v); jw_end(&jw); jw_end(&jw); diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index 58d9e430f05..6e429a3fb9e 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -232,7 +232,7 @@ static void fn_child_start_fl(const char *file, int line, strbuf_addch(&buf_payload, ' '); if (cmd->git_cmd) strbuf_addstr(&buf_payload, "git "); - sq_append_quote_argv_pretty(&buf_payload, cmd->argv); + sq_append_quote_argv_pretty(&buf_payload, cmd->args.v); normal_io_write_fl(file, line, &buf_payload); strbuf_release(&buf_payload); diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index e4acca13d64..2ff9cf70835 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -335,10 +335,10 @@ static void fn_child_start_fl(const char *file, int line, strbuf_addstr(&buf_payload, " argv:["); if (cmd->git_cmd) { strbuf_addstr(&buf_payload, "git"); - if (cmd->argv[0]) + if (cmd->args.nr) strbuf_addch(&buf_payload, ' '); } - sq_append_quote_argv_pretty(&buf_payload, cmd->argv); + sq_append_quote_argv_pretty(&buf_payload, cmd->args.v); strbuf_addch(&buf_payload, ']'); perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute, From patchwork Thu Nov 25 22:52:23 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: 12639949 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0722CC433EF for ; Thu, 25 Nov 2021 22:56:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346344AbhKYW7s (ORCPT ); Thu, 25 Nov 2021 17:59:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57988 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237441AbhKYW5m (ORCPT ); Thu, 25 Nov 2021 17:57:42 -0500 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CBDACC061759 for ; Thu, 25 Nov 2021 14:52:36 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id p27-20020a05600c1d9b00b0033bf8532855so5617228wms.3 for ; Thu, 25 Nov 2021 14:52:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=yOZ5YvM9Q3uXmM4Q0Gps7urCZmFpfS/Czr2Jg81l35Q=; b=ZUNdYQ9T5bWIKifojklWVSCaWXYDTSLuMjZucqwBFC781DW0AWXBJ+iQMAYfrL/zYp zUtE7tcyAqAWu04hDHgv6NfptNfNi26IWgUTpeEi4mfHxYY+5rk2KnkVU5DU2Pu+/bsQ OzV/46N7O5/SKNKP63Sv5FD4UkiRHRm0TkTqKasUASqqzxLb+qATK52hOBnnWOLAAtuj zdZLSAEcszkROsL5dm6kSyRkhOaO1zp3schB3Ai+/8AB/9ZYrusukNLom4yKHcLU6WLy lIBtmVpO27qZSmMxbZ/SBw4lrTRf5L4xKfAcyawY6KOtZh600XYeP2UE1zqQfInFmpv+ NobA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=yOZ5YvM9Q3uXmM4Q0Gps7urCZmFpfS/Czr2Jg81l35Q=; b=DALwbfgnvvxq+FWnwOd3sbuZkBE5FNE2gZ0D1piLOjK+0sVWPHwU03FjpnqgSKiIu8 2ufB+7v15VR0tnlosYh2XoiD2K8TT2Zb+hnVbaHOluScEluKAUiUZPu05FOBXxHMFG2R M2P0A0Ryxj148ITXNX4QfxUVgMIsKVxu9caVQJS5se13xz+OvXpU4UupsvvE0lOXZwKJ AJPJT8d+Ktmzg4xXnQLTg8LhNg26tw9LldJxMdqO9bNzakEL18Ijd6Z449dXTavuzz+q w+N9zrLsx1WrqnalVzrJh5CgUI9m3RxGaISv81UEOhO2ybO6Ua9aSdRq+Xw991M3hBK6 o56Q== X-Gm-Message-State: AOAM5311PvEaVx+ZSNJBGtXY9lgxqvQJHDNXu/l9UlaPmRIvvvNiICBi uz5m1bT5AwSkGay0Z7wjDdenuqmiOzUfww== X-Google-Smtp-Source: ABdhPJw3bcDc1Jfts+xJkzsD/IOGrOj2eww4dIZ14ErPEtaKJK5oLdukVL7Uq8ndSeVGWFd3gWQySw== X-Received: by 2002:a1c:a74a:: with SMTP id q71mr11960572wme.105.1637880755188; Thu, 25 Nov 2021 14:52:35 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:34 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 8/9] difftool: use "env_array" to simplify memory management Date: Thu, 25 Nov 2021 23:52:23 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Amend code added in 03831ef7b50 (difftool: implement the functionality in the builtin, 2017-01-19) to use the "env_array" in the run_command.[ch] API. Now we no longer need to manage our own "index_env" buffer. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/difftool.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index 4931c108451..4ee40fe3a06 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -202,15 +202,10 @@ static void changed_files(struct hashmap *result, const char *index_path, { struct child_process update_index = CHILD_PROCESS_INIT; struct child_process diff_files = CHILD_PROCESS_INIT; - struct strbuf index_env = STRBUF_INIT, buf = STRBUF_INIT; - const char *git_dir = absolute_path(get_git_dir()), *env[] = { - NULL, NULL - }; + struct strbuf buf = STRBUF_INIT; + const char *git_dir = absolute_path(get_git_dir()); FILE *fp; - strbuf_addf(&index_env, "GIT_INDEX_FILE=%s", index_path); - env[0] = index_env.buf; - strvec_pushl(&update_index.args, "--git-dir", git_dir, "--work-tree", workdir, "update-index", "--really-refresh", "-q", @@ -222,7 +217,7 @@ static void changed_files(struct hashmap *result, const char *index_path, update_index.use_shell = 0; update_index.clean_on_exit = 1; update_index.dir = workdir; - update_index.env = env; + strvec_pushf(&update_index.env_array, "GIT_INDEX_FILE=%s", index_path); /* Ignore any errors of update-index */ run_command(&update_index); @@ -235,7 +230,7 @@ static void changed_files(struct hashmap *result, const char *index_path, diff_files.clean_on_exit = 1; diff_files.out = -1; diff_files.dir = workdir; - diff_files.env = env; + strvec_pushf(&diff_files.env_array, "GIT_INDEX_FILE=%s", index_path); if (start_command(&diff_files)) die("could not obtain raw diff"); fp = xfdopen(diff_files.out, "r"); @@ -248,7 +243,6 @@ static void changed_files(struct hashmap *result, const char *index_path, fclose(fp); if (finish_command(&diff_files)) die("diff-files did not exit properly"); - strbuf_release(&index_env); strbuf_release(&buf); } From patchwork Thu Nov 25 22:52:24 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: 12639951 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1961BC4332F for ; Thu, 25 Nov 2021 22:56:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346414AbhKYW7t (ORCPT ); Thu, 25 Nov 2021 17:59:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57994 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237543AbhKYW5n (ORCPT ); Thu, 25 Nov 2021 17:57:43 -0500 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E05B4C06175A for ; Thu, 25 Nov 2021 14:52:37 -0800 (PST) Received: by mail-wr1-x42f.google.com with SMTP id r8so14561598wra.7 for ; Thu, 25 Nov 2021 14:52:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=an4921mEsiUx4HsKaUVGgTkVKenK01kI8nyRI400NTc=; b=m0x6H0/HtUSQAXAV2d0Y913GhXCrdL09Frn/j9q243pTVV0nuabM04FCmekQnltiNX dvPFlIHEBRINmFGG5v1XSQR3nYiCJFSbM8e62bII4zHfpcKJ0M9+6rQQIXK0dRpNdCzy 65P2ofdvgqapOWuTJbALenIVdz9dEuFBSa3fi2BmBrx2QVvjeclCUk1spTylKDIyRm5w 1rr3gZo55mi2p/WG1d5Xfjz0H8Qe0nwgo9alOxPjaWR2g3szG7yGU//2pQwvJ/CcpwdM k5Fk8Z1D/W+l2+t9ZHWQdNu2MIIvO02m3su/GvdPjRLMqPmy/OpD/OyUF1BTRZox/903 Kllg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=an4921mEsiUx4HsKaUVGgTkVKenK01kI8nyRI400NTc=; b=h4Q15O6TNO06eQNkj1JnJ5+g0zaYFdwilSXVuhJchCrKdCs5HX4LzCoBExC/tYezne sBvEtfIa3czR07u0+4O0Fackx4i6Y4hv0225690xRdkSvybf0dAX2ERLnwYbv3MjKhBO Ub3WlRes5iYMte1JA9U4s20uJMKxZEnhKuZheX8X3lCKvPe3GvwydRCgfQu+kZnKIta7 MfieVsZnwVBcBHoFWgOcRLIKahiyDwmI4zuB2fFJusbCc68RLYUH2wp2IKicR1u3SrBA Kp8MymKY6ivcjd/bPVlHSwkbe5IBWpOm/yL6PO1fSad4JTXiwDGhwd6hXdAxIzJY1dLl qDkw== X-Gm-Message-State: AOAM532DbrnCqKgLNTMmrwWDwX6upNo+bHNatC2B5GfXEvEUkLyE0EH0 /82/vFKN48XrXvGCiTkOtnTu+MUxj9jMIw== X-Google-Smtp-Source: ABdhPJwks1RGZlL3GCc51wSyjiBt8dWE6wCpuSR3f+RYo1uXtpwRn7K+XgcVi8h1vidxxHlPHeUvmg== X-Received: by 2002:a05:6000:1aca:: with SMTP id i10mr10586526wry.407.1637880756152; Thu, 25 Nov 2021 14:52:36 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id h22sm5001257wmq.14.2021.11.25.14.52.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 14:52:35 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Enzo Matsumiya , Eric Sunshine , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 9/9] run-command API: remove "env" member, always use "env_array" Date: Thu, 25 Nov 2021 23:52:24 +0100 Message-Id: X-Mailer: git-send-email 2.34.1.838.g779e9098efb In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Remove the "env" member from "struct child_process" in favor of always using the "env_array". As with the preceding removal of "argv" in favor of "args" this gets rid of current and future oddities around memory management at the API boundary (see the amended API docs). For some of the conversions we can replace patterns like: child.env = env->v; With: strvec_pushv(&child.env_array, env->v); But for others we need to guard the strvec_pushv() with a NULL check, since we're not passing in the "v" member of a "struct strvec", e.g. in the case of tmp_objdir_env()'s return value. Ideally we'd rename the "env_array" member to simply "env" as a follow-up, since it and "args" are now inconsistent in not having an "_array" suffix, and seemingly without any good reason, unless we look at the history of how they came to be. But as we've currently got 122 in-tree hits for a "git grep env_array" let's leave that for now (and possibly forever). Doing that rename would be too disruptive. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/receive-pack.c | 11 ++++++----- builtin/worktree.c | 6 +++--- connected.c | 3 ++- editor.c | 4 +++- object-file.c | 2 +- run-command.c | 20 +++++++------------- run-command.h | 34 +++++++++++++++++----------------- trailer.c | 2 +- 8 files changed, 40 insertions(+), 42 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 48c99c8ee45..3979752ceca 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1367,7 +1367,7 @@ static const char *push_to_deploy(unsigned char *sha1, strvec_pushl(&child.args, "update-index", "-q", "--ignore-submodules", "--refresh", NULL); - child.env = env->v; + strvec_pushv(&child.env_array, env->v); child.dir = work_tree; child.no_stdin = 1; child.stdout_to_stderr = 1; @@ -1379,7 +1379,7 @@ static const char *push_to_deploy(unsigned char *sha1, child_process_init(&child); strvec_pushl(&child.args, "diff-files", "--quiet", "--ignore-submodules", "--", NULL); - child.env = env->v; + strvec_pushv(&child.env_array, env->v); child.dir = work_tree; child.no_stdin = 1; child.stdout_to_stderr = 1; @@ -1393,7 +1393,7 @@ static const char *push_to_deploy(unsigned char *sha1, /* diff-index with either HEAD or an empty tree */ head_has_history() ? "HEAD" : empty_tree_oid_hex(), "--", NULL); - child.env = env->v; + strvec_pushv(&child.env_array, env->v); child.no_stdin = 1; child.no_stdout = 1; child.stdout_to_stderr = 0; @@ -1404,7 +1404,7 @@ static const char *push_to_deploy(unsigned char *sha1, child_process_init(&child); strvec_pushl(&child.args, "read-tree", "-u", "-m", hash_to_hex(sha1), NULL); - child.env = env->v; + strvec_pushv(&child.env_array, env->v); child.dir = work_tree; child.no_stdin = 1; child.no_stdout = 1; @@ -2202,7 +2202,8 @@ static const char *unpack(int err_fd, struct shallow_info *si) close(err_fd); return "unable to create temporary object directory"; } - child.env = tmp_objdir_env(tmp_objdir); + if (tmp_objdir) + strvec_pushv(&child.env_array, tmp_objdir_env(tmp_objdir)); /* * Normally we just pass the tmp_objdir environment to the child diff --git a/builtin/worktree.c b/builtin/worktree.c index 9edd3e2829b..962d71cf987 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -349,7 +349,7 @@ static int add_worktree(const char *path, const char *refname, strvec_push(&cp.args, "--quiet"); } - cp.env = child_env.v; + strvec_pushv(&cp.env_array, child_env.v); ret = run_command(&cp); if (ret) goto done; @@ -360,7 +360,7 @@ static int add_worktree(const char *path, const char *refname, strvec_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL); if (opts->quiet) strvec_push(&cp.args, "--quiet"); - cp.env = child_env.v; + strvec_pushv(&cp.env_array, child_env.v); ret = run_command(&cp); if (ret) goto done; @@ -389,7 +389,7 @@ static int add_worktree(const char *path, const char *refname, cp.no_stdin = 1; cp.stdout_to_stderr = 1; cp.dir = path; - cp.env = env; + strvec_pushv(&cp.env_array, env); cp.trace2_hook_name = "post-checkout"; strvec_pushl(&cp.args, absolute_path(hook), oid_to_hex(null_oid()), diff --git a/connected.c b/connected.c index 35bd4a26382..ed3025e7a2a 100644 --- a/connected.c +++ b/connected.c @@ -109,7 +109,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data, _("Checking connectivity")); rev_list.git_cmd = 1; - rev_list.env = opt->env; + if (opt->env) + strvec_pushv(&rev_list.env_array, opt->env); rev_list.in = -1; rev_list.no_stdout = 1; if (opt->err_fd) diff --git a/editor.c b/editor.c index d92a8d9ab5b..8b9648281d7 100644 --- a/editor.c +++ b/editor.c @@ -1,6 +1,7 @@ #include "cache.h" #include "config.h" #include "strbuf.h" +#include "strvec.h" #include "run-command.h" #include "sigchain.h" @@ -78,7 +79,8 @@ static int launch_specified_editor(const char *editor, const char *path, strbuf_realpath(&realpath, path, 1); strvec_pushl(&p.args, editor, realpath.buf, NULL); - p.env = env; + if (env) + strvec_pushv(&p.env_array, (const char **)env); p.use_shell = 1; p.trace2_child_class = "editor"; if (start_command(&p) < 0) { diff --git a/object-file.c b/object-file.c index c3d866a287e..2fc1bed417c 100644 --- a/object-file.c +++ b/object-file.c @@ -797,7 +797,7 @@ static void fill_alternate_refs_command(struct child_process *cmd, } } - cmd->env = local_repo_env; + strvec_pushv(&cmd->env_array, (const char **)local_repo_env); cmd->out = -1; } diff --git a/run-command.c b/run-command.c index 99dc93e7300..ebade086700 100644 --- a/run-command.c +++ b/run-command.c @@ -655,12 +655,7 @@ static void trace_run_command(const struct child_process *cp) sq_quote_buf_pretty(&buf, cp->dir); strbuf_addch(&buf, ';'); } - /* - * The caller is responsible for initializing cp->env from - * cp->env_array if needed. We only check one place. - */ - if (cp->env) - trace_add_env(&buf, cp->env); + trace_add_env(&buf, cp->env_array.v); if (cp->git_cmd) strbuf_addstr(&buf, " git"); sq_quote_argv_pretty(&buf, cp->args.v); @@ -676,9 +671,6 @@ int start_command(struct child_process *cmd) int failed_errno; char *str; - if (!cmd->env) - cmd->env = cmd->env_array.v; - /* * In case of errors we must keep the promise to close FDs * that have been passed in via ->in and ->out. @@ -768,7 +760,7 @@ int start_command(struct child_process *cmd) set_cloexec(null_fd); } - childenv = prep_childenv(cmd->env); + childenv = prep_childenv(cmd->env_array.v); atfork_prepare(&as); /* @@ -931,7 +923,7 @@ int start_command(struct child_process *cmd) else if (cmd->use_shell) cmd->args.v = prepare_shell_cmd(&nargv, sargv); - cmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v, (char**) cmd->env, + cmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v, (char**) cmd->env_array.v, cmd->dir, fhin, fhout, fherr); failed_errno = errno; if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT)) @@ -1047,7 +1039,8 @@ int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir, cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0; cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0; cmd.dir = dir; - cmd.env = env; + if (env) + strvec_pushv(&cmd.env_array, (const char **)env); cmd.trace2_child_class = tr2_class; return run_command(&cmd); } @@ -1333,7 +1326,8 @@ int run_hook_ve(const char *const *env, const char *name, va_list args) strvec_push(&hook.args, p); while ((p = va_arg(args, const char *))) strvec_push(&hook.args, p); - hook.env = env; + if (env) + strvec_pushv(&hook.env_array, (const char **)env); hook.no_stdin = 1; hook.stdout_to_stderr = 1; hook.trace2_hook_name = name; diff --git a/run-command.h b/run-command.h index c0d1210cc63..2be5f5d6422 100644 --- a/run-command.h +++ b/run-command.h @@ -56,6 +56,23 @@ struct child_process { * `finish_command` (or during `start_command` when it is unsuccessful). */ struct strvec args; + + /** + * Like .args the .env_array is a `struct strvec'. + * + * To modify the environment of the sub-process, specify an array of + * environment settings. Each string in the array manipulates the + * environment. + * + * - If the string is of the form "VAR=value", i.e. it contains '=' + * the variable is added to the child process's environment. + * + * - If the string does not contain '=', it names an environment + * variable that will be removed from the child process's environment. + * + * The memory in .env_array will be cleaned up automatically during + * `finish_command` (or during `start_command` when it is unsuccessful). + */ struct strvec env_array; pid_t pid; @@ -92,23 +109,6 @@ struct child_process { */ const char *dir; - /** - * To modify the environment of the sub-process, specify an array of - * string pointers (NULL terminated) in .env: - * - * - If the string is of the form "VAR=value", i.e. it contains '=' - * the variable is added to the child process's environment. - * - * - If the string does not contain '=', it names an environment - * variable that will be removed from the child process's environment. - * - * If the .env member is NULL, `start_command` will point it at the - * .env_array `strvec` (so you may use one or the other, but not both). - * The memory in .env_array will be cleaned up automatically during - * `finish_command` (or during `start_command` when it is unsuccessful). - */ - const char *const *env; - unsigned no_stdin:1; unsigned no_stdout:1; unsigned no_stderr:1; diff --git a/trailer.c b/trailer.c index 7c7cb61a945..1b12f77d945 100644 --- a/trailer.c +++ b/trailer.c @@ -236,7 +236,7 @@ static char *apply_command(struct conf_info *conf, const char *arg) strbuf_replace(&cmd, TRAILER_ARG_STRING, arg); strvec_push(&cp.args, cmd.buf); } - cp.env = local_repo_env; + strvec_pushv(&cp.env_array, (const char **)local_repo_env); cp.no_stdin = 1; cp.use_shell = 1;