Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> GUARD_PATHSPEC() marks pathspec-sensitive code (basically anything in
> 'struct pathspec' except fields "nr" and "original"). GUARD_PATHSPEC()
> is not supposed to fail. The steps for a new pathspec magic or
> optimization would be:
>
> - update parse_pathspec, add extra information to struct pathspec
>
> - grep GUARD_PATHSPEC() and update all relevant code (or note those
> that won't work with your new stuff). Update GUARD_PATHSPEC mask
> accordingly.
>
> - update parse_pathspec calls to disable new magic early at command
> parsing level. Make sure parse_pathspec() catches unsupported
> syntax, not until GUARD_PATHSPEC catches it.
>
> - add tests to verify supported/unsupported commands both work as
> expected.
I think Documentation/technical/api-*.txt wants to have all of the
above. It is not clear from the description what the second bitmask
is supposed to mean, without reading the implementation; I am guessing
that you are supposed to list the kinds of magic that are supported
in the codepath?
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> builtin/diff.c | 2 ++
> dir.c | 2 ++
> pathspec.h | 7 +++++++
> tree-diff.c | 19 +++++++++++++++++++
> tree-walk.c | 2 ++
> 5 files changed, 32 insertions(+)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 8c2af6c..d237e0a 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -371,6 +371,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> die(_("unhandled object '%s' given."), name);
> }
> if (rev.prune_data.nr) {
> + /* builtin_diff_b_f() */
> + GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
> if (!path)
> path = rev.prune_data.items[0].match;
> paths += rev.prune_data.nr;
> diff --git a/dir.c b/dir.c
> index 1e9db41..6094ba8 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -297,6 +297,8 @@ int match_pathspec_depth(const struct pathspec *ps,
> {
> int i, retval = 0;
>
> + GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
> +
> if (!ps->nr) {
> if (!ps->recursive ||
> !(ps->magic & PATHSPEC_MAXDEPTH) ||
> diff --git a/pathspec.h b/pathspec.h
> index 1cef9c6..ed5d3a6 100644
> --- a/pathspec.h
> +++ b/pathspec.h
> @@ -27,6 +27,13 @@ struct pathspec {
> } *items;
> };
>
> +#define GUARD_PATHSPEC(ps, mask) \
> + do { \
> + if ((ps)->magic & ~(mask)) \
> + die("BUG:%s:%d: unsupported magic %x", \
> + __FILE__, __LINE__, (ps)->magic & ~(mask)); \
> + } while (0)
> +
> /* parse_pathspec flags */
> #define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
> #define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
> diff --git a/tree-diff.c b/tree-diff.c
> index 826512e..5a87614 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -198,6 +198,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
> const char *paths[1];
> int i;
>
> + /*
> + * follow-rename code is very specific, we need exactly one
> + * path. Magic that matches more than one path is not
> + * supported.
> + */
> + GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP);
> +#if 0
> + /*
> + * We should reject wildcards as well. Unfortunately we
> + * haven't got a reliable way to detect that 'foo\*bar' in
> + * fact has no wildcards. nowildcard_len is merely a hint for
> + * optimization. Let it slip for now until wildmatch is taught
> + * about dry-run mode and returns wildcard info.
> + */
> + if (opt->pathspec.has_wildcard)
> + die("BUG:%s:%d: wildcards are not supported",
> + __FILE__, __LINE__);
> +#endif
> +
> /* Remove the file creation entry from the diff queue, and remember it */
> choice = q->queue[0];
> q->nr = 0;
> diff --git a/tree-walk.c b/tree-walk.c
> index d399ca9..37b157e 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -636,6 +636,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
> enum interesting never_interesting = ps->has_wildcard ?
> entry_not_interesting : all_entries_not_interesting;
>
> + GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
> +
> if (!ps->nr) {
> if (!ps->recursive ||
> !(ps->magic & PATHSPEC_MAXDEPTH) ||
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html