From 87323bdace47c3d464a783f48b16617720e8eeee Mon Sep 17 00:00:00 2001 From: Nguyễn Thái Ngọc Duy Date: Sun, 14 Jul 2013 15:35:28 +0700 Subject: add parse_pathspec() that converts cmdline args to struct pathspec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently to fill a struct pathspec, we do: const char **paths; paths = get_pathspec(prefix, argv); ... init_pathspec(&pathspec, paths); "paths" can only carry bare strings, which loses information from command line arguments such as pathspec magic or the prefix part's length for each argument. parse_pathspec() is introduced to combine the two calls into one. The plan is gradually replace all get_pathspec() and init_pathspec() with parse_pathspec(). get_pathspec() now becomes a thin wrapper of parse_pathspec(). parse_pathspec() allows the caller to reject the pathspec magics that it does not support. When a new pathspec magic is introduced, we can enable it per command after making sure that all underlying code has no problem with the new magic. "flags" parameter is currently unused. But it would allow callers to pass certain instructions to parse_pathspec, for example forcing literal pathspec when no magic is used. With the introduction of parse_pathspec, there are now two functions that can initialize struct pathspec: init_pathspec and parse_pathspec. Any semantic changes in struct pathspec must be reflected in both functions. init_pathspec() will be phased out in favor of parse_pathspec(). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/api-setup.txt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'Documentation/technical') diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt index 4f63a04d7d..90d1aff625 100644 --- a/Documentation/technical/api-setup.txt +++ b/Documentation/technical/api-setup.txt @@ -8,6 +8,23 @@ Talk about * is_inside_git_dir() * is_inside_work_tree() * setup_work_tree() -* get_pathspec() (Dscho) + +Pathspec +-------- + +See glossary-context.txt for the syntax of pathspec. In memory, a +pathspec set is represented by "struct pathspec" and is prepared by +parse_pathspec(). This function takes several arguments: + +- magic_mask specifies what features that are NOT supported by the + following code. If a user attempts to use such a feature, + parse_pathspec() can reject it early. + +- flags specifies other things that the caller wants parse_pathspec to + perform. + +- prefix and args come from cmd_* functions + +get_pathspec() is obsolete and should never be used in new code. -- cgit v1.3-5-g9baa From 8f4f8f4579fe8cc630e118cc736de2b8d5cf8e34 Mon Sep 17 00:00:00 2001 From: Nguyễn Thái Ngọc Duy Date: Sun, 14 Jul 2013 15:35:36 +0700 Subject: guard against new pathspec magic in pathspec matching code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GUARD_PATHSPEC() marks pathspec-sensitive code, basically all those that touch anything in 'struct pathspec' except fields "nr" and "original". GUARD_PATHSPEC() is not supposed to fail. It's mainly to help the designers catch unsupported codepaths. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/api-setup.txt | 19 +++++++++++++++++++ builtin/diff.c | 2 ++ dir.c | 2 ++ pathspec.h | 7 +++++++ tree-diff.c | 19 +++++++++++++++++++ tree-walk.c | 2 ++ 6 files changed, 51 insertions(+) (limited to 'Documentation/technical') diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt index 90d1aff625..540e455689 100644 --- a/Documentation/technical/api-setup.txt +++ b/Documentation/technical/api-setup.txt @@ -28,3 +28,22 @@ parse_pathspec(). This function takes several arguments: - prefix and args come from cmd_* functions get_pathspec() is obsolete and should never be used in new code. + +parse_pathspec() helps catch unsupported features and reject them +politely. At a lower level, different pathspec-related functions may +not support the same set of features. Such pathspec-sensitive +functions are guarded with GUARD_PATHSPEC(), which will die in an +unfriendly way when an unsupported feature is requested. + +The command designers are supposed to make sure that GUARD_PATHSPEC() +never dies. They have to make sure all unsupported features are caught +by parse_pathspec(), not by GUARD_PATHSPEC. grepping GUARD_PATHSPEC() +should give the designers all pathspec-sensitive codepaths and what +features they support. + +A similar process is applied when a new pathspec magic is added. The +designer lifts the GUARD_PATHSPEC restriction in the functions that +support the new magic. At the same time (s)he has to make sure this +new feature will be caught at parse_pathspec() in commands that cannot +handle the new magic in some cases. grepping parse_pathspec() should +help. diff --git a/builtin/diff.c b/builtin/diff.c index 9fc273d8cd..6bb41aff5d 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -367,6 +367,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) } } 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 e28bc0d636..19978d3d59 100644 --- a/dir.c +++ b/dir.c @@ -340,6 +340,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 2e427d7f4c..6baf205862 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 826512e84d..5a876148bb 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 d399ca9dcc..37b157ed6e 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) || -- cgit v1.3-5-g9baa