guard against new pathspec magic in pathspec matching code
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 <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Jul 14, 2013 at 15:35 UTC
8f4f8f4579fe8cc630e118cc736de2b8d5cf8e34
6 files changed
+51
Documentation/technical/api-setup.txt
+19
@@ -28,3 +28,22 @@ parse_pathspec(). This function takes several arguments:
28
- prefix and args come from cmd_* functions
29
30
get_pathspec() is obsolete and should never be used in new code.
31
+
32
+parse_pathspec() helps catch unsupported features and reject them
33
+politely. At a lower level, different pathspec-related functions may
34
+not support the same set of features. Such pathspec-sensitive
35
+functions are guarded with GUARD_PATHSPEC(), which will die in an
36
+unfriendly way when an unsupported feature is requested.
37
+
38
+The command designers are supposed to make sure that GUARD_PATHSPEC()
39
+never dies. They have to make sure all unsupported features are caught
40
+by parse_pathspec(), not by GUARD_PATHSPEC. grepping GUARD_PATHSPEC()
41
+should give the designers all pathspec-sensitive codepaths and what
42
+features they support.
43
+
44
+A similar process is applied when a new pathspec magic is added. The
45
+designer lifts the GUARD_PATHSPEC restriction in the functions that
46
+support the new magic. At the same time (s)he has to make sure this
47
+new feature will be caught at parse_pathspec() in commands that cannot
48
+handle the new magic in some cases. grepping parse_pathspec() should
49
+help.
builtin/diff.c
+2
@@ -367,6 +367,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
367
}
368
}
369
if (rev.prune_data.nr) {
370
+ /* builtin_diff_b_f() */
371
+ GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
372
if (!path)
373
path = rev.prune_data.items[0].match;
374
paths += rev.prune_data.nr;
dir.c
+2
@@ -340,6 +340,8 @@ int match_pathspec_depth(const struct pathspec *ps,
340
{
341
int i, retval = 0;
342
343
+ GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
344
+
345
if (!ps->nr) {
346
if (!ps->recursive ||
347
!(ps->magic & PATHSPEC_MAXDEPTH) ||
pathspec.h
+7
@@ -27,6 +27,13 @@ struct pathspec {
27
} *items;
28
};
29
30
+#define GUARD_PATHSPEC(ps, mask) \
31
+ do { \
32
+ if ((ps)->magic & ~(mask)) \
33
+ die("BUG:%s:%d: unsupported magic %x", \
34
+ __FILE__, __LINE__, (ps)->magic & ~(mask)); \
35
+ } while (0)
36
+
37
/* parse_pathspec flags */
38
#define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
39
#define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
tree-diff.c
+19
@@ -198,6 +198,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
198
const char *paths[1];
199
int i;
200
201
+ /*
202
+ * follow-rename code is very specific, we need exactly one
203
+ * path. Magic that matches more than one path is not
204
+ * supported.
205
+ */
206
+ GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP);
207
+#if 0
208
+ /*
209
+ * We should reject wildcards as well. Unfortunately we
210
+ * haven't got a reliable way to detect that 'foo\*bar' in
211
+ * fact has no wildcards. nowildcard_len is merely a hint for
212
+ * optimization. Let it slip for now until wildmatch is taught
213
+ * about dry-run mode and returns wildcard info.
214
+ */
215
+ if (opt->pathspec.has_wildcard)
216
+ die("BUG:%s:%d: wildcards are not supported",
217
+ __FILE__, __LINE__);
218
+#endif
219
+
220
/* Remove the file creation entry from the diff queue, and remember it */
221
choice = q->queue[0];
222
q->nr = 0;
tree-walk.c
+2
@@ -636,6 +636,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
636
enum interesting never_interesting = ps->has_wildcard ?
637
entry_not_interesting : all_entries_not_interesting;
638
639
+ GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
640
+
641
if (!ps->nr) {
642
if (!ps->recursive ||
643
!(ps->magic & PATHSPEC_MAXDEPTH) ||