git: refactor builtin handling to use a `struct strvec`
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@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Nov 20, 2024 at 14:39 UTC
1dd7c32daa7d8a4b71d9204f1edbb09a7241e18f
2 files changed
+32
-36
git.c
+31
-35
@@ -696,63 +696,57 @@ void load_builtin_commands(const char *prefix, struct cmdnames *cmds)
696
}
697
698
#ifdef STRIP_EXTENSION
699
-static void strip_extension(const char **argv)
699
+static void strip_extension(struct strvec *args)
700
{
701
size_t len;
702
703
- if (strip_suffix(argv[0], STRIP_EXTENSION, &len))
704
- argv[0] = xmemdupz(argv[0], len);
703
+ if (strip_suffix(args->v[0], STRIP_EXTENSION, &len)) {
704
+ char *stripped = xmemdupz(args->v[0], len);
705
+ strvec_replace(args, 0, stripped);
706
+ free(stripped);
707
+ }
708
}
709
#else
710
#define strip_extension(cmd)
711
#endif
712
710
-static void handle_builtin(int argc, const char **argv)
713
+static void handle_builtin(struct strvec *args)
714
{
712
- struct strvec args = STRVEC_INIT;
713
- const char **argv_copy = NULL;
715
const char *cmd;
716
struct cmd_struct *builtin;
717
717
- strip_extension(argv);
718
- cmd = argv[0];
718
+ strip_extension(args);
719
+ cmd = args->v[0];
720
721
/* Turn "git cmd --help" into "git help --exclude-guides cmd" */
721
- if (argc > 1 && !strcmp(argv[1], "--help")) {
722
- int i;
723
-
724
- argv[1] = argv[0];
725
- argv[0] = cmd = "help";
726
-
727
- for (i = 0; i < argc; i++) {
728
- strvec_push(&args, argv[i]);
729
- if (!i)
730
- strvec_push(&args, "--exclude-guides");
731
- }
722
+ if (args->nr > 1 && !strcmp(args->v[1], "--help")) {
723
+ const char *exclude_guides_arg[] = { "--exclude-guides" };
724
+
725
+ strvec_replace(args, 1, args->v[0]);
726
+ strvec_replace(args, 0, "help");
727
+ cmd = "help";
728
+ strvec_splice(args, 2, 0, exclude_guides_arg,
729
+ ARRAY_SIZE(exclude_guides_arg));
730
+ }
731
733
- argc++;
732
+ builtin = get_builtin(cmd);
733
+ if (builtin) {
734
+ const char **argv_copy = NULL;
735
+ int ret;
736
737
/*
738
* `run_builtin()` will modify the argv array, so we need to
739
* create a shallow copy such that we can free all of its
740
* strings.
741
*/
740
- CALLOC_ARRAY(argv_copy, argc + 1);
741
- COPY_ARRAY(argv_copy, args.v, argc);
742
+ if (args->nr)
743
+ DUP_ARRAY(argv_copy, args->v, args->nr + 1);
744
743
- argv = argv_copy;
744
- }
745
-
746
- builtin = get_builtin(cmd);
747
- if (builtin) {
748
- int ret = run_builtin(builtin, argc, argv, the_repository);
749
- strvec_clear(&args);
745
+ ret = run_builtin(builtin, args->nr, argv_copy, the_repository);
746
+ strvec_clear(args);
747
free(argv_copy);
748
exit(ret);
749
}
753
-
754
- strvec_clear(&args);
755
- free(argv_copy);
750
}
751
752
static void execv_dashed_external(const char **argv)
@@ -815,7 +809,7 @@ static int run_argv(struct strvec *args)
809
* process.
810
*/
811
if (!done_alias)
818
- handle_builtin(args->nr, args->v);
812
+ handle_builtin(args);
813
else if (get_builtin(args->v[0])) {
814
struct child_process cmd = CHILD_PROCESS_INIT;
815
int i;
@@ -916,8 +910,10 @@ int cmd_main(int argc, const char **argv)
910
* that one cannot handle it.
911
*/
912
if (skip_prefix(cmd, "git-", &cmd)) {
919
- argv[0] = cmd;
920
- handle_builtin(argc, argv);
913
+ strvec_push(&args, cmd);
914
+ strvec_pushv(&args, argv + 1);
915
+ handle_builtin(&args);
916
+ strvec_clear(&args);
917
die(_("cannot handle %s as a builtin"), cmd);
918
}
919
t/t0211-trace2-perf.sh
+1
-1
@@ -2,7 +2,7 @@
2
3
test_description='test trace2 facility (perf target)'
4
5
-TEST_PASSES_SANITIZE_LEAK=false
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
# Turn off any inherited trace2 settings for this test.