From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
"René Scharfe" <l.s.r@web.de>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 10/10] run-command API: add and use a run_command_sv_opt()
Date: Mon, 17 Oct 2022 19:49:21 +0200 [thread overview]
Message-ID: <patch-v2-10.10-138af632a36-20221017T170316Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v2-00.10-00000000000-20221017T170316Z-avarab@gmail.com>
Add a run_command_sv_opt() convenience wrapper for
run_command_v_opt(), as noted in the API documentation this is for the
common case of wanting to construct a "struct strvec" to pass to
run_command_v_opt(), and as it's a one-shot to strvec_clear() it
afterwards.
Let's convert those API users that were using a "ret" variable to
carry over to "return" after a "strvec_clear()" to use this new
function instead.
Because we pass the "struct strvec *" to the function we can also
avoid copying the arguments to the "args" member of the "struct
child_process", as we were doing with run_command_v_opt().
Instead we can use memcpy() and strvec_clear() to do the moral
equivalent of a strbuf_{detach,attach}(). The strvec API doesn't have
a strvec_attach(), we could add it here while at it, but let's avoid
generalizing the interface for now and migrate the "struct strvec *"
in the "run_command_sv_opt()" instead.
Let's leave aside the user in "builtin/bisect--helper.c"'s
bisect_visualize(). There's an outstanding topic that's extensively
modifying it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
add-interactive.c | 3 +--
builtin/add.c | 6 ++----
builtin/clone.c | 3 +--
builtin/gc.c | 5 +----
builtin/pull.c | 15 +++------------
builtin/remote.c | 5 +----
merge.c | 3 +--
run-command.c | 11 +++++++++++
run-command.h | 16 +++++++++++++++-
scalar.c | 6 +-----
sequencer.c | 11 ++---------
11 files changed, 39 insertions(+), 45 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index f071b2a1b4f..9c86f3b9532 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1007,8 +1007,7 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps,
if (files->selected[i])
strvec_push(&args,
files->items.items[i].string);
- res = run_command_v_opt(args.v, 0);
- strvec_clear(&args);
+ res = run_command_sv_opt(&args, 0);
}
putchar('\n');
diff --git a/builtin/add.c b/builtin/add.c
index f84372964c8..7c783eebc0e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -240,7 +240,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)
int run_add_interactive(const char *revision, const char *patch_mode,
const struct pathspec *pathspec)
{
- int status, i;
+ int i;
struct strvec argv = STRVEC_INIT;
int use_builtin_add_i =
git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
@@ -282,9 +282,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,
/* pass original pathspec, to be re-parsed */
strvec_push(&argv, pathspec->items[i].original);
- status = run_command_v_opt(argv.v, RUN_GIT_CMD);
- strvec_clear(&argv);
- return status;
+ return run_command_sv_opt(&argv, RUN_GIT_CMD);
}
int interactive_add(const char **argv, const char *prefix, int patch)
diff --git a/builtin/clone.c b/builtin/clone.c
index a1514363190..8e43781e147 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -756,8 +756,7 @@ static int checkout(int submodule_progress, int filter_submodules)
"--single-branch" :
"--no-single-branch");
- err = run_command_v_opt(args.v, RUN_GIT_CMD);
- strvec_clear(&args);
+ return run_command_sv_opt(&args, RUN_GIT_CMD);
}
return err;
diff --git a/builtin/gc.c b/builtin/gc.c
index 519e64e86ee..8393e19b504 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1899,7 +1899,6 @@ static int is_schtasks_available(void)
static int schtasks_remove_task(enum schedule_priority schedule)
{
const char *cmd = "schtasks";
- int result;
struct strvec args = STRVEC_INIT;
const char *frequency = get_frequency(schedule);
@@ -1909,9 +1908,7 @@ static int schtasks_remove_task(enum schedule_priority schedule)
strvec_pushf(&args, SCHTASKS_NAME_FMT, frequency);
strvec_pushl(&args, "/f", NULL);
- result = run_command_v_opt(args.v, 0);
- strvec_clear(&args);
- return result;
+ return run_command_sv_opt(&args, 0);
}
static int schtasks_remove_tasks(void)
diff --git a/builtin/pull.c b/builtin/pull.c
index 403a24d7ca6..2f36823c97e 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -516,7 +516,6 @@ static void parse_repo_refspecs(int argc, const char **argv, const char **repo,
static int run_fetch(const char *repo, const char **refspecs)
{
struct strvec args = STRVEC_INIT;
- int ret;
strvec_pushl(&args, "fetch", "--update-head-ok", NULL);
@@ -582,9 +581,7 @@ static int run_fetch(const char *repo, const char **refspecs)
strvec_pushv(&args, refspecs);
} else if (*refspecs)
BUG("refspecs without repo?");
- ret = run_command_v_opt(args.v, RUN_GIT_CMD | RUN_CLOSE_OBJECT_STORE);
- strvec_clear(&args);
- return ret;
+ return run_command_sv_opt(&args, RUN_GIT_CMD | RUN_CLOSE_OBJECT_STORE);
}
/**
@@ -653,7 +650,6 @@ static int update_submodules(void)
*/
static int run_merge(void)
{
- int ret;
struct strvec args = STRVEC_INIT;
strvec_pushl(&args, "merge", NULL);
@@ -696,9 +692,7 @@ static int run_merge(void)
strvec_push(&args, "--allow-unrelated-histories");
strvec_push(&args, "FETCH_HEAD");
- ret = run_command_v_opt(args.v, RUN_GIT_CMD);
- strvec_clear(&args);
- return ret;
+ return run_command_sv_opt(&args, RUN_GIT_CMD);
}
/**
@@ -879,7 +873,6 @@ static int get_rebase_newbase_and_upstream(struct object_id *newbase,
static int run_rebase(const struct object_id *newbase,
const struct object_id *upstream)
{
- int ret;
struct strvec args = STRVEC_INIT;
strvec_push(&args, "rebase");
@@ -913,9 +906,7 @@ static int run_rebase(const struct object_id *newbase,
strvec_push(&args, oid_to_hex(upstream));
- ret = run_command_v_opt(args.v, RUN_GIT_CMD);
- strvec_clear(&args);
- return ret;
+ return run_command_sv_opt(&args, RUN_GIT_CMD);
}
static int get_can_ff(struct object_id *orig_head,
diff --git a/builtin/remote.c b/builtin/remote.c
index 910f7b9316a..5d3534db19c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1510,7 +1510,6 @@ static int update(int argc, const char **argv, const char *prefix)
};
struct strvec fetch_argv = STRVEC_INIT;
int default_defined = 0;
- int retval;
argc = parse_options(argc, argv, prefix, options,
builtin_remote_update_usage,
@@ -1536,9 +1535,7 @@ static int update(int argc, const char **argv, const char *prefix)
}
}
- retval = run_command_v_opt(fetch_argv.v, RUN_GIT_CMD);
- strvec_clear(&fetch_argv);
- return retval;
+ return run_command_sv_opt(&fetch_argv, RUN_GIT_CMD);
}
static int remove_all_fetch_refspecs(const char *key)
diff --git a/merge.c b/merge.c
index 2382ff66d35..487debacecb 100644
--- a/merge.c
+++ b/merge.c
@@ -33,8 +33,7 @@ int try_merge_command(struct repository *r,
for (j = remotes; j; j = j->next)
strvec_push(&args, merge_argument(j->item));
- ret = run_command_v_opt(args.v, RUN_GIT_CMD);
- strvec_clear(&args);
+ ret = run_command_sv_opt(&args, RUN_GIT_CMD);
discard_index(r->index);
if (repo_read_index(r) < 0)
diff --git a/run-command.c b/run-command.c
index 0066ace85fa..724fc581c89 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1038,6 +1038,17 @@ int run_command_v_opt(const char **argv, int opt)
return run_command(&cmd);
}
+int run_command_sv_opt(struct strvec *args, int opt)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+
+ /* TODO: We could encapsulate this with a strvec_attach() */
+ memcpy(&cmd.args, args, sizeof(*args));
+ strvec_init(args);
+ run_command_set_opts(&cmd, opt);
+ return run_command(&cmd);
+}
+
#ifndef NO_PTHREADS
static pthread_t main_thread;
static int main_thread_set;
diff --git a/run-command.h b/run-command.h
index 2b1fe3cde5c..8e8403f3bb3 100644
--- a/run-command.h
+++ b/run-command.h
@@ -151,7 +151,7 @@ struct child_process {
/**
* The functions: child_process_init, start_command, finish_command,
- * run_command, run_command_l_opt, run_command_v_opt,
+ * run_command, run_command_l_opt, run_command_v_opt, run_command_sv_opt,
* child_process_clear do the following:
*
* - If a system call failed, errno is set and -1 is returned. A diagnostic
@@ -262,6 +262,20 @@ int run_command_v_opt(const char **argv, int opt);
LAST_ARG_MUST_BE_NULL
int run_command_l_opt(int opt, ...);
+/**
+ * The run_command_sv_opt() function is a wrapper for
+ * run_command_v_opt(). It takes a "struct strvec *args" which
+ * similarly will be strvec_clear()'d before returning.
+ *
+ * Use it for the common case of constructing a "struct strvec" for a
+ * one-shot run_command_v_opt() invocation.
+ *
+ * The "args" will migrated the "cmd.args" member of an underlying
+ * "struct child_process", in a way that avoids making an extra copy.
+ */
+RESULT_MUST_BE_USED
+int run_command_sv_opt(struct strvec *args, int opt);
+
/**
* Execute the given command, sending "in" to its stdin, and capturing its
* stdout and stderr in the "out" and "err" strbufs. Any of the three may
diff --git a/scalar.c b/scalar.c
index 6de9c0ee523..3480bf73cbd 100644
--- a/scalar.c
+++ b/scalar.c
@@ -72,7 +72,6 @@ static int run_git(const char *arg, ...)
struct strvec argv = STRVEC_INIT;
va_list args;
const char *p;
- int res;
va_start(args, arg);
strvec_push(&argv, arg);
@@ -80,10 +79,7 @@ static int run_git(const char *arg, ...)
strvec_push(&argv, p);
va_end(args);
- res = run_command_v_opt(argv.v, RUN_GIT_CMD);
-
- strvec_clear(&argv);
- return res;
+ return run_command_sv_opt(&argv, RUN_GIT_CMD);
}
struct scalar_config {
diff --git a/sequencer.c b/sequencer.c
index 20495db9de2..7ee0e05512c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3183,7 +3183,6 @@ static int rollback_is_safe(void)
static int reset_merge(const struct object_id *oid)
{
- int ret;
struct strvec argv = STRVEC_INIT;
strvec_pushl(&argv, "reset", "--merge", NULL);
@@ -3191,10 +3190,7 @@ static int reset_merge(const struct object_id *oid)
if (!is_null_oid(oid))
strvec_push(&argv, oid_to_hex(oid));
- ret = run_command_v_opt(argv.v, RUN_GIT_CMD);
- strvec_clear(&argv);
-
- return ret;
+ return run_command_sv_opt(&argv, RUN_GIT_CMD);
}
static int rollback_single_pick(struct repository *r)
@@ -4866,7 +4862,6 @@ static int pick_commits(struct repository *r,
static int continue_single_pick(struct repository *r, struct replay_opts *opts)
{
struct strvec argv = STRVEC_INIT;
- int ret;
if (!refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
!refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD"))
@@ -4887,9 +4882,7 @@ static int continue_single_pick(struct repository *r, struct replay_opts *opts)
*/
strvec_pushl(&argv, "--no-edit", "--cleanup=strip", NULL);
- ret = run_command_v_opt(argv.v, RUN_GIT_CMD);
- strvec_clear(&argv);
- return ret;
+ return run_command_sv_opt(&argv, RUN_GIT_CMD);
}
static int commit_staged_changes(struct repository *r,
--
2.38.0.1091.gf9d18265e59
next prev parent reply other threads:[~2022-10-17 17:50 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-14 15:40 [PATCH 00/10] run-command API: add run_command_{l,sv}_opt() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers Ævar Arnfjörð Bjarmason
2022-10-14 18:22 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 02/10] merge: remove always-the-same "verbose" arguments Ævar Arnfjörð Bjarmason
2022-10-14 18:31 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 03/10] run-command API: add and use a run_command_l_opt() Ævar Arnfjörð Bjarmason
2022-10-14 18:40 ` Junio C Hamano
2022-10-14 20:03 ` Jeff King
2022-10-14 20:27 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 04/10] am: use run_command_l_opt() for show_patch() Ævar Arnfjörð Bjarmason
2022-10-14 18:43 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 05/10] run-command API docs: clarify & fleshen out run_command_v_opt*() docs Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 06/10] run-command API: remove RUN_COMMAND_STDOUT_TO_STDERR flag Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 07/10] run-command API & diff.c: remove run_command_v_opt_cd_env() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 08/10] run-command API & users: remove run_command_v_opt_tr2() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 09/10] gc: use strvec_pushf(), avoid redundant strbuf_detach() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 10/10] run-command API: add and use a run_command_sv_opt() Ævar Arnfjörð Bjarmason
2022-10-14 19:21 ` René Scharfe
2022-10-17 17:49 ` [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 02/10] merge: remove always-the-same "verbose" arguments Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 03/10] run-command API: add and use a run_command_l_opt() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 04/10] am: use run_command_l_opt() for show_patch() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 05/10] run-command API docs: clarify & fleshen out run_command_v_opt*() docs Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 06/10] run-command API: remove RUN_COMMAND_STDOUT_TO_STDERR flag Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 07/10] run-command API & diff.c: remove run_command_v_opt_cd_env() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 08/10] run-command API & users: remove run_command_v_opt_tr2() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 09/10] gc: use strvec_pushf(), avoid redundant strbuf_detach() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` Ævar Arnfjörð Bjarmason [this message]
2022-10-18 9:11 ` [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt() Junio C Hamano
2022-10-18 13:28 ` Ævar Arnfjörð Bjarmason
2022-10-18 20:42 ` Jeff King
2022-10-19 15:43 ` Junio C Hamano
2022-10-19 17:06 ` Jeff King
2022-10-19 18:00 ` Junio C Hamano
2022-10-19 18:57 ` Jeff King
2022-10-19 19:41 ` Junio C Hamano
2022-10-20 18:34 ` René Scharfe
2022-10-20 23:50 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-v2-10.10-138af632a36-20221017T170316Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).