diff --git a/bisect.c b/bisect.c index f24474542e..a6dc76b15c 100644 --- a/bisect.c +++ b/bisect.c @@ -674,9 +674,6 @@ static void bisect_rev_setup(struct repository *r, struct rev_info *revs, const char *bad_format, const char *good_format, int read_paths) { - struct setup_revision_opt opt = { - .free_removed_argv_elements = 1, - }; int i; repo_init_revisions(r, revs, prefix); @@ -693,7 +690,7 @@ static void bisect_rev_setup(struct repository *r, struct rev_info *revs, if (read_paths) read_bisect_paths(rev_argv); - setup_revisions(rev_argv->nr, rev_argv->v, revs, &opt); + setup_revisions_from_strvec(rev_argv, revs, NULL); } static void bisect_common(struct rev_info *revs) diff --git a/builtin/describe.c b/builtin/describe.c index 9f4e26d7ff..ffaf8d9f0a 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -580,7 +580,8 @@ static void describe_blob(const struct object_id *oid, struct strbuf *dst) NULL); repo_init_revisions(the_repository, &revs, NULL); - if (setup_revisions(args.nr, args.v, &revs, NULL) > 1) + setup_revisions_from_strvec(&args, &revs, NULL); + if (args.nr > 1) BUG("setup_revisions could not handle all args?"); if (prepare_revision_walk(&revs)) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1494afcf3d..5856b5f6bf 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4650,7 +4650,7 @@ static void get_object_list_path_walk(struct rev_info *revs) die(_("failed to pack objects via path-walk")); } -static void get_object_list(struct rev_info *revs, int ac, const char **av) +static void get_object_list(struct rev_info *revs, struct strvec *argv) { struct setup_revision_opt s_r_opt = { .allow_exclude_promisor_objects = 1, @@ -4660,7 +4660,7 @@ static void get_object_list(struct rev_info *revs, int ac, const char **av) int save_warning; save_commit_buffer = 0; - setup_revisions(ac, av, revs, &s_r_opt); + setup_revisions_from_strvec(argv, revs, &s_r_opt); /* make sure shallows are read */ is_repository_shallow(the_repository); @@ -5229,7 +5229,7 @@ int cmd_pack_objects(int argc, revs.include_check = is_not_in_promisor_pack; revs.include_check_obj = is_not_in_promisor_pack_obj; } - get_object_list(&revs, rp.nr, rp.v); + get_object_list(&revs, &rp); release_revisions(&revs); } cleanup_preferred_base(); diff --git a/builtin/rebase.c b/builtin/rebase.c index 67c0352bf8..c468828189 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -299,8 +299,7 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) oid_to_hex(&opts->restrict_revision->object.oid)); ret = sequencer_make_script(the_repository, &todo_list.buf, - make_script_args.nr, make_script_args.v, - flags); + &make_script_args, flags); if (ret) { error(_("could not generate todo list")); goto cleanup; diff --git a/builtin/stash.c b/builtin/stash.c index b7db7c8364..bea64d04a0 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1015,8 +1015,8 @@ static int show_stash(int argc, const char **argv, const char *prefix, } } - argc = setup_revisions(revision_args.nr, revision_args.v, &rev, NULL); - if (argc > 1) + setup_revisions_from_strvec(&revision_args, &rev, NULL); + if (revision_args.nr > 1) goto usage; if (!rev.diffopt.output_format) { rev.diffopt.output_format = DIFF_FORMAT_PATCH; diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 07a1935cbe..fcd73abe53 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -616,9 +616,6 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, struct rev_info rev = REV_INFO_INIT; struct strbuf buf = STRBUF_INIT; const char *git_dir; - struct setup_revision_opt opt = { - .free_removed_argv_elements = 1, - }; if (validate_submodule_path(path) < 0) die(NULL); @@ -655,7 +652,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, repo_init_revisions(the_repository, &rev, NULL); rev.abbrev = 0; - setup_revisions(diff_files_args.nr, diff_files_args.v, &rev, &opt); + setup_revisions_from_strvec(&diff_files_args, &rev, NULL); run_diff_files(&rev, 0); if (!diff_result_code(&rev)) { @@ -1094,9 +1091,6 @@ static int compute_summary_module_list(struct object_id *head_oid, { struct strvec diff_args = STRVEC_INIT; struct rev_info rev; - struct setup_revision_opt opt = { - .free_removed_argv_elements = 1, - }; struct module_cb_list list = MODULE_CB_LIST_INIT; int ret = 0; @@ -1114,7 +1108,7 @@ static int compute_summary_module_list(struct object_id *head_oid, repo_init_revisions(the_repository, &rev, info->prefix); rev.abbrev = 0; precompose_argv_prefix(diff_args.nr, diff_args.v, NULL); - setup_revisions(diff_args.nr, diff_args.v, &rev, &opt); + setup_revisions_from_strvec(&diff_args, &rev, NULL); rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK; rev.diffopt.format_callback = submodule_summary_callback; rev.diffopt.format_callback_data = &list; diff --git a/http-push.c b/http-push.c index 91a5465afb..4c43ba3bc7 100644 --- a/http-push.c +++ b/http-push.c @@ -1941,7 +1941,7 @@ int cmd_main(int argc, const char **argv) strvec_pushf(&commit_argv, "^%s", oid_to_hex(&ref->old_oid)); repo_init_revisions(the_repository, &revs, setup_git_directory()); - setup_revisions(commit_argv.nr, commit_argv.v, &revs, NULL); + setup_revisions_from_strvec(&commit_argv, &revs, NULL); revs.edge_hint = 0; /* just in case */ /* Generate a list of objects that need to be pushed */ diff --git a/remote.c b/remote.c index 81d8fc017e..df9675cd33 100644 --- a/remote.c +++ b/remote.c @@ -2143,9 +2143,6 @@ static int stat_branch_pair(const char *branch_name, const char *base, struct object_id oid; struct commit *ours, *theirs; struct rev_info revs; - struct setup_revision_opt opt = { - .free_removed_argv_elements = 1, - }; struct strvec argv = STRVEC_INIT; /* Cannot stat if what we used to build on no longer exists */ @@ -2180,7 +2177,7 @@ static int stat_branch_pair(const char *branch_name, const char *base, strvec_push(&argv, "--"); repo_init_revisions(the_repository, &revs, NULL); - setup_revisions(argv.nr, argv.v, &revs, &opt); + setup_revisions_from_strvec(&argv, &revs, NULL); if (prepare_revision_walk(&revs)) die(_("revision walk setup failed")); diff --git a/revision.c b/revision.c index 6ba8f67054..806a1c4c24 100644 --- a/revision.c +++ b/revision.c @@ -2321,6 +2321,24 @@ static timestamp_t parse_age(const char *arg) return num; } +static void overwrite_argv(int *argc, const char **argv, + const char **value, + const struct setup_revision_opt *opt) +{ + /* + * Detect the case when we are overwriting ourselves. The assignment + * itself would be a noop either way, but this lets us avoid corner + * cases around the free() and NULL operations. + */ + if (*value != argv[*argc]) { + if (opt && opt->free_removed_argv_elements) + free((char *)argv[*argc]); + argv[*argc] = *value; + *value = NULL; + } + (*argc)++; +} + static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv, int *unkc, const char **unkv, const struct setup_revision_opt* opt) @@ -2342,7 +2360,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg starts_with(arg, "--branches=") || starts_with(arg, "--tags=") || starts_with(arg, "--remotes=") || starts_with(arg, "--no-walk=")) { - unkv[(*unkc)++] = arg; + overwrite_argv(unkc, unkv, &argv[0], opt); return 1; } @@ -2706,7 +2724,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg } else { int opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix); if (!opts) - unkv[(*unkc)++] = arg; + overwrite_argv(unkc, unkv, &argv[0], opt); return opts; } @@ -3018,7 +3036,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (!strcmp(arg, "--stdin")) { if (revs->disable_stdin) { - argv[left++] = arg; + overwrite_argv(&left, argv, &argv[i], opt); continue; } if (revs->read_from_stdin++) @@ -3174,9 +3192,34 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s revs->show_notes_given = 1; } + if (argv) { + if (opt && opt->free_removed_argv_elements) + free((char *)argv[left]); + argv[left] = NULL; + } + return left; } +void setup_revisions_from_strvec(struct strvec *argv, struct rev_info *revs, + struct setup_revision_opt *opt) +{ + struct setup_revision_opt fallback_opt; + int ret; + + if (!opt) { + memset(&fallback_opt, 0, sizeof(fallback_opt)); + opt = &fallback_opt; + } + opt->free_removed_argv_elements = 1; + + ret = setup_revisions(argv->nr, argv->v, revs, opt); + + for (size_t i = ret; i < argv->nr; i++) + free((char *)argv->v[i]); + argv->nr = ret; +} + static void release_revisions_cmdline(struct rev_cmdline_info *cmdline) { unsigned int i; diff --git a/revision.h b/revision.h index 21e288c5ba..a28e349044 100644 --- a/revision.h +++ b/revision.h @@ -441,6 +441,8 @@ struct setup_revision_opt { }; int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *); +void setup_revisions_from_strvec(struct strvec *argv, struct rev_info *revs, + struct setup_revision_opt *); /** * Free data allocated in a "struct rev_info" after it's been diff --git a/sequencer.c b/sequencer.c index 6d29a938aa..5476d39ba9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -6052,8 +6052,8 @@ static int make_script_with_merges(struct pretty_print_context *pp, return 0; } -int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, - const char **argv, unsigned flags) +int sequencer_make_script(struct repository *r, struct strbuf *out, + struct strvec *argv, unsigned flags) { char *format = NULL; struct pretty_print_context pp = {0}; @@ -6094,7 +6094,8 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, pp.fmt = revs.commit_format; pp.output_encoding = get_log_output_encoding(); - if (setup_revisions(argc, argv, &revs, NULL) > 1) { + setup_revisions_from_strvec(argv, &revs, NULL); + if (argv->nr > 1) { ret = error(_("make_script: unhandled options")); goto cleanup; } diff --git a/sequencer.h b/sequencer.h index 304ba4b4d3..719684c8a9 100644 --- a/sequencer.h +++ b/sequencer.h @@ -186,8 +186,8 @@ int sequencer_remove_state(struct replay_opts *opts); #define TODO_LIST_REAPPLY_CHERRY_PICKS (1U << 7) #define TODO_LIST_WARN_SKIPPED_CHERRY_PICKS (1U << 8) -int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, - const char **argv, unsigned flags); +int sequencer_make_script(struct repository *r, struct strbuf *out, + struct strvec *argv, unsigned flags); int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags, const char *shortrevisions, const char *onto_name, diff --git a/shallow.c b/shallow.c index ef3adb635f..d9cd4e219c 100644 --- a/shallow.c +++ b/shallow.c @@ -213,7 +213,7 @@ static void show_commit(struct commit *commit, void *data) * are marked with shallow_flag. The list of border/shallow commits * are also returned. */ -struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av, +struct commit_list *get_shallow_commits_by_rev_list(struct strvec *argv, int shallow_flag, int not_shallow_flag) { @@ -232,7 +232,7 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av, repo_init_revisions(the_repository, &revs, NULL); save_commit_buffer = 0; - setup_revisions(ac, av, &revs, NULL); + setup_revisions_from_strvec(argv, &revs, NULL); if (prepare_revision_walk(&revs)) die("revision walk setup failed"); diff --git a/shallow.h b/shallow.h index 9bfeade93e..ad591bd139 100644 --- a/shallow.h +++ b/shallow.h @@ -7,6 +7,7 @@ #include "strbuf.h" struct oid_array; +struct strvec; void set_alternate_shallow_file(struct repository *r, const char *path, int override); int register_shallow(struct repository *r, const struct object_id *oid); @@ -36,8 +37,8 @@ void rollback_shallow_file(struct repository *r, struct shallow_lock *lk); struct commit_list *get_shallow_commits(struct object_array *heads, int depth, int shallow_flag, int not_shallow_flag); -struct commit_list *get_shallow_commits_by_rev_list( - int ac, const char **av, int shallow_flag, int not_shallow_flag); +struct commit_list *get_shallow_commits_by_rev_list(struct strvec *argv, + int shallow_flag, int not_shallow_flag); int write_shallow_commits(struct strbuf *out, int use_pack_protocol, const struct oid_array *extra); diff --git a/submodule.c b/submodule.c index fff3c75570..35c55155f7 100644 --- a/submodule.c +++ b/submodule.c @@ -900,7 +900,7 @@ static void collect_changed_submodules(struct repository *r, save_warning = warn_on_object_refname_ambiguity; warn_on_object_refname_ambiguity = 0; repo_init_revisions(r, &rev, NULL); - setup_revisions(argv->nr, argv->v, &rev, &s_r_opt); + setup_revisions_from_strvec(argv, &rev, &s_r_opt); warn_on_object_refname_ambiguity = save_warning; if (prepare_revision_walk(&rev)) die(_("revision walk setup failed")); diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 0bb4648e36..930c31e547 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1741,4 +1741,13 @@ test_expect_success 'submodules does not affect the branch recorded in stash mes ) ' +test_expect_success SANITIZE_LEAK 'stash show handles -- without leaking' ' + git stash show -- +' + +test_expect_success 'controlled error return on unrecognized option' ' + test_expect_code 129 git stash show -p --invalid 2>usage && + grep -e "^usage: git stash show" usage +' + test_done diff --git a/upload-pack.c b/upload-pack.c index f78fabc1e1..1e87ae9559 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -913,13 +913,12 @@ static void deepen(struct upload_pack_data *data, int depth) } static void deepen_by_rev_list(struct upload_pack_data *data, - int ac, - const char **av) + struct strvec *argv) { struct commit_list *result; disable_commit_graph(the_repository); - result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW); + result = get_shallow_commits_by_rev_list(argv, SHALLOW, NOT_SHALLOW); send_shallow(data, result); free_commit_list(result); send_unshallow(data); @@ -955,7 +954,7 @@ static int send_shallow_list(struct upload_pack_data *data) struct object *o = data->want_obj.objects[i].item; strvec_push(&av, oid_to_hex(&o->oid)); } - deepen_by_rev_list(data, av.nr, av.v); + deepen_by_rev_list(data, &av); strvec_clear(&av); ret = 1; } else {