git: refactor alias handling to use a `struct strvec`
In `handle_alias()` we use both `argcp` and `argv` as in-out parameters. Callers mostly pass through the static array from `main()`, but once we handle an alias we replace it with an allocated array that may contain some allocated strings. Callers do not handle this scenario at all and thus leak memory. We could in theory handle the lifetime of `argv` in a hacky fashion by letting callers free it in case they see that an alias was handled. But while that would likely work, we still wouldn't be able to easily handle the lifetime of strings referenced by `argv`. Refactor the code to instead use a `struct strvec`, which effectively removes the need for us to manually track lifetimes. 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
ffc5c046fb4f47e149687963706bb2390f4eed61
2 files changed
+33
-26
git.c
+32
-26
@@ -362,7 +362,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
362
return (*argv) - orig_argv;
363
}
364
365
-static int handle_alias(int *argcp, const char ***argv)
365
+static int handle_alias(struct strvec *args)
366
{
367
int envchanged = 0, ret = 0, saved_errno = errno;
368
int count, option_count;
@@ -370,10 +370,10 @@ static int handle_alias(int *argcp, const char ***argv)
370
const char *alias_command;
371
char *alias_string;
372
373
- alias_command = (*argv)[0];
373
+ alias_command = args->v[0];
374
alias_string = alias_lookup(alias_command);
375
if (alias_string) {
376
- if (*argcp > 1 && !strcmp((*argv)[1], "-h"))
376
+ if (args->nr > 1 && !strcmp(args->v[1], "-h"))
377
fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
378
alias_command, alias_string);
379
if (alias_string[0] == '!') {
@@ -390,7 +390,7 @@ static int handle_alias(int *argcp, const char ***argv)
390
child.wait_after_clean = 1;
391
child.trace2_child_class = "shell_alias";
392
strvec_push(&child.args, alias_string + 1);
393
- strvec_pushv(&child.args, (*argv) + 1);
393
+ strvec_pushv(&child.args, args->v + 1);
394
395
trace2_cmd_alias(alias_command, child.args.v);
396
trace2_cmd_name("_run_shell_alias_");
@@ -423,15 +423,13 @@ static int handle_alias(int *argcp, const char ***argv)
423
trace_argv_printf(new_argv,
424
"trace: alias expansion: %s =>",
425
alias_command);
426
-
427
- REALLOC_ARRAY(new_argv, count + *argcp);
428
- /* insert after command name */
429
- COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
430
-
426
trace2_cmd_alias(alias_command, new_argv);
427
433
- *argv = new_argv;
434
- *argcp += count - 1;
428
+ /* Replace the alias with the new arguments. */
429
+ strvec_splice(args, 0, 1, new_argv, count);
430
+
431
+ free(alias_string);
432
+ free(new_argv);
433
434
ret = 1;
435
}
@@ -800,10 +798,10 @@ static void execv_dashed_external(const char **argv)
798
exit(128);
799
}
800
803
-static int run_argv(int *argcp, const char ***argv)
801
+static int run_argv(struct strvec *args)
802
{
803
int done_alias = 0;
806
- struct string_list cmd_list = STRING_LIST_INIT_NODUP;
804
+ struct string_list cmd_list = STRING_LIST_INIT_DUP;
805
struct string_list_item *seen;
806
807
while (1) {
@@ -817,8 +815,8 @@ static int run_argv(int *argcp, const char ***argv)
815
* process.
816
*/
817
if (!done_alias)
820
- handle_builtin(*argcp, *argv);
821
- else if (get_builtin(**argv)) {
818
+ handle_builtin(args->nr, args->v);
819
+ else if (get_builtin(args->v[0])) {
820
struct child_process cmd = CHILD_PROCESS_INIT;
821
int i;
822
@@ -834,8 +832,8 @@ static int run_argv(int *argcp, const char ***argv)
832
commit_pager_choice();
833
834
strvec_push(&cmd.args, "git");
837
- for (i = 0; i < *argcp; i++)
838
- strvec_push(&cmd.args, (*argv)[i]);
835
+ for (i = 0; i < args->nr; i++)
836
+ strvec_push(&cmd.args, args->v[i]);
837
838
trace_argv_printf(cmd.args.v, "trace: exec:");
839
@@ -850,13 +848,13 @@ static int run_argv(int *argcp, const char ***argv)
848
i = run_command(&cmd);
849
if (i >= 0 || errno != ENOENT)
850
exit(i);
853
- die("could not execute builtin %s", **argv);
851
+ die("could not execute builtin %s", args->v[0]);
852
}
853
854
/* .. then try the external ones */
857
- execv_dashed_external(*argv);
855
+ execv_dashed_external(args->v);
856
859
- seen = unsorted_string_list_lookup(&cmd_list, *argv[0]);
857
+ seen = unsorted_string_list_lookup(&cmd_list, args->v[0]);
858
if (seen) {
859
int i;
860
struct strbuf sb = STRBUF_INIT;
@@ -873,14 +871,14 @@ static int run_argv(int *argcp, const char ***argv)
871
" not terminate:%s"), cmd_list.items[0].string, sb.buf);
872
}
873
876
- string_list_append(&cmd_list, *argv[0]);
874
+ string_list_append(&cmd_list, args->v[0]);
875
876
/*
877
* It could be an alias -- this works around the insanity
878
* of overriding "git log" with "git show" by having
879
* alias.log = show
880
*/
883
- if (!handle_alias(argcp, argv))
881
+ if (!handle_alias(args))
882
break;
883
done_alias = 1;
884
}
@@ -892,6 +890,7 @@ static int run_argv(int *argcp, const char ***argv)
890
891
int cmd_main(int argc, const char **argv)
892
{
893
+ struct strvec args = STRVEC_INIT;
894
const char *cmd;
895
int done_help = 0;
896
@@ -951,25 +950,32 @@ int cmd_main(int argc, const char **argv)
950
*/
951
setup_path();
952
953
+ for (size_t i = 0; i < argc; i++)
954
+ strvec_push(&args, argv[i]);
955
+
956
while (1) {
955
- int was_alias = run_argv(&argc, &argv);
957
+ int was_alias = run_argv(&args);
958
if (errno != ENOENT)
959
break;
960
if (was_alias) {
961
fprintf(stderr, _("expansion of alias '%s' failed; "
962
"'%s' is not a git command\n"),
961
- cmd, argv[0]);
963
+ cmd, args.v[0]);
964
+ strvec_clear(&args);
965
exit(1);
966
}
967
if (!done_help) {
965
- cmd = argv[0] = help_unknown_cmd(cmd);
968
+ strvec_replace(&args, 0, help_unknown_cmd(cmd));
969
+ cmd = args.v[0];
970
done_help = 1;
967
- } else
971
+ } else {
972
break;
973
+ }
974
}
975
976
fprintf(stderr, _("failed to run command '%s': %s\n"),
977
cmd, strerror(errno));
978
+ strvec_clear(&args);
979
980
return 1;
981
}
t/t0014-alias.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='git command aliasing'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_expect_success 'nested aliases - internal execution' '