Similar as with the preceding commit, `handle_builtin()` does not
properly track lifetimes of the `argv` array and its strings. As it may
end up modifying the array this can lead to memory leaks in case it
contains allocated strings.
Refactor the function to use a `struct strvec` instead.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
git.c | 66 ++++++++++++++++++++++++--------------------------
t/t0211-trace2-perf.sh | 2 +-
2 files changed, 32 insertions(+), 36 deletions(-)
diff --git a/git.c b/git.c
index 88356afe5fb568ccc147f055e3ab253c53a1befa..159dd45b08204c4a89d1dc4ab6990978e2454eb6 100644
--- a/git.c
+++ b/git.c
@@ -696,63 +696,57 @@ void load_builtin_commands(const char *prefix, struct cmdnames *cmds)
}
#ifdef STRIP_EXTENSION
-static void strip_extension(const char **argv)
+static void strip_extension(struct strvec *args)
{
size_t len;
- if (strip_suffix(argv[0], STRIP_EXTENSION, &len))
- argv[0] = xmemdupz(argv[0], len);
+ if (strip_suffix(args->v[0], STRIP_EXTENSION, &len)) {
+ char *stripped = xmemdupz(args->v[0], len);
+ strvec_replace(args, 0, stripped);
+ free(stripped);
+ }
}
#else
#define strip_extension(cmd)
#endif
-static void handle_builtin(int argc, const char **argv)
+static void handle_builtin(struct strvec *args)
{
- struct strvec args = STRVEC_INIT;
- const char **argv_copy = NULL;
const char *cmd;
struct cmd_struct *builtin;
- strip_extension(argv);
- cmd = argv[0];
+ strip_extension(args);
+ cmd = args->v[0];
/* Turn "git cmd --help" into "git help --exclude-guides cmd" */
- if (argc > 1 && !strcmp(argv[1], "--help")) {
- int i;
-
- argv[1] = argv[0];
- argv[0] = cmd = "help";
-
- for (i = 0; i < argc; i++) {
- strvec_push(&args, argv[i]);
- if (!i)
- strvec_push(&args, "--exclude-guides");
- }
+ if (args->nr > 1 && !strcmp(args->v[1], "--help")) {
+ const char *exclude_guides_arg[] = { "--exclude-guides" };
+
+ strvec_replace(args, 1, args->v[0]);
+ strvec_replace(args, 0, "help");
+ cmd = "help";
+ strvec_splice(args, 2, 0, exclude_guides_arg,
+ ARRAY_SIZE(exclude_guides_arg));
+ }
- argc++;
+ builtin = get_builtin(cmd);
+ if (builtin) {
+ const char **argv_copy = NULL;
+ int ret;
/*
* `run_builtin()` will modify the argv array, so we need to
* create a shallow copy such that we can free all of its
* strings.
*/
- CALLOC_ARRAY(argv_copy, argc + 1);
- COPY_ARRAY(argv_copy, args.v, argc);
+ if (args->nr)
+ DUP_ARRAY(argv_copy, args->v, args->nr + 1);
- argv = argv_copy;
- }
-
- builtin = get_builtin(cmd);
- if (builtin) {
- int ret = run_builtin(builtin, argc, argv, the_repository);
- strvec_clear(&args);
+ ret = run_builtin(builtin, args->nr, argv_copy, the_repository);
+ strvec_clear(args);
free(argv_copy);
exit(ret);
}
-
- strvec_clear(&args);
- free(argv_copy);
}
static void execv_dashed_external(const char **argv)
@@ -815,7 +809,7 @@ static int run_argv(struct strvec *args)
* process.
*/
if (!done_alias)
- handle_builtin(args->nr, args->v);
+ handle_builtin(args);
else if (get_builtin(args->v[0])) {
struct child_process cmd = CHILD_PROCESS_INIT;
int i;
@@ -916,8 +910,10 @@ int cmd_main(int argc, const char **argv)
* that one cannot handle it.
*/
if (skip_prefix(cmd, "git-", &cmd)) {
- argv[0] = cmd;
- handle_builtin(argc, argv);
+ strvec_push(&args, cmd);
+ strvec_pushv(&args, argv + 1);
+ handle_builtin(&args);
+ strvec_clear(&args);
die(_("cannot handle %s as a builtin"), cmd);
}
diff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh
index dddc130560ba6150fc5f5eac36c65ff76a2d31a1..91260ce97e5bdb39550a9e1e4abbc4d5fea48a21 100755
--- a/t/t0211-trace2-perf.sh
+++ b/t/t0211-trace2-perf.sh
@@ -2,7 +2,7 @@
test_description='test trace2 facility (perf target)'
-TEST_PASSES_SANITIZE_LEAK=false
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
# Turn off any inherited trace2 settings for this test.
--
2.47.0.229.g8f8d6eee53.dirty
