Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:
> Call clear_pathspec() on the pathspec initialized in
> push_stash().
This puzzled me for a while. This patch adds an {0} initializer to the
declaration of the pathspec. I assumed that this is necessary to avoid
giving clear_pathspec() an uninitialized struct. It isn't, though,
because the pathspec is handed to parse_pathspec() first, which
initializes it. So you can safely drop the first hunk.
> Initializing that structure in this way is already done
> by a few other callers, and now we have:
>
> $ git grep -e 'struct pathspec.* = { 0 }' -e memset.pathspec
> add-interactive.c: struct pathspec ps_selected = { 0 };
> builtin/sparse-checkout.c: struct pathspec p = { 0 };
> builtin/stash.c: struct pathspec ps = { 0 };
> pathspec.c: memset(pathspec, 0, sizeof(*pathspec));
> wt-status.c: struct pathspec ps = { 0 };
Not sure if this part of the commit message is useful. It seems to
suggest that the only place to initialize a pathspec with memset is
pathspec.c itself, but there are a few more. Here's a really sloppy
way to find (some of?) them:
$ git grep -e 'struct pathspec [^*]' -e memset --all-match -p -n | awk '
/struct pathspec [^*]/ {
decl=$0
declfunc=prevfunc
var=decl; sub(/^.* /, "", var); sub(/;/, "", var)
next
}
/memset/ && declfunc==prevfunc && match($0, var) {
print decl
print
next
}
{prevfunc=$0}'
builtin/log.c:726: struct pathspec match_all;
builtin/log.c:737: memset(&match_all, 0, sizeof(match_all));
builtin/ls-files.c:572: struct pathspec pathspec;
builtin/ls-files.c:600: memset(&pathspec, 0, sizeof(pathspec));
builtin/stash.c:1469: struct pathspec ps;
builtin/stash.c:1474: memset(&ps, 0, sizeof(ps));
builtin/stash.c:1787: struct pathspec ps;
builtin/stash.c:1813: memset(&ps, 0, sizeof(ps));
merge-recursive.c:479: struct pathspec match_all;
merge-recursive.c:480: memset(&match_all, 0, sizeof(match_all));
sparse-index.c:364: struct pathspec ps;
sparse-index.c:388: memset(&ps, 0, sizeof(ps));
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> builtin/stash.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index bb0fd861434..e82fb69c2b3 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1708,7 +1708,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> int pathspec_file_nul = 0;
> const char *stash_msg = NULL;
> const char *pathspec_from_file = NULL;
> - struct pathspec ps;
> + struct pathspec ps = { 0 };
> struct option options[] = {
> OPT_BOOL('k', "keep-index", &keep_index,
> N_("keep index")),
> @@ -1727,6 +1727,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
> OPT_END()
> };
> + int ret;
>
> if (argc) {
> force_assume = !strcmp(argv[0], "-p");
> @@ -1766,8 +1767,10 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
> }
>
> - return do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode,
> - include_untracked, only_staged);
> + ret = do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode,
> + include_untracked, only_staged);
> + clear_pathspec(&ps);
> + return ret;
> }
>
> static int push_stash_unassumed(int argc, const char **argv, const char *prefix)
