From 0de1633783685e9fb1943551217cdda7edbd245b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2011 17:36:09 +1100 Subject: tree-walk.c: do not leak internal structure in tree_entry_len() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tree_entry_len() does not simply take two random arguments and return a tree length. The two pointers must point to a tree item structure, or struct name_entry. Passing random pointers will return incorrect value. Force callers to pass struct name_entry instead of two pointers (with hope that they don't manually construct struct name_entry themselves) Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/grep.c | 2 +- builtin/pack-objects.c | 2 +- tree-diff.c | 6 +++--- tree-walk.c | 16 ++++++++-------- tree-walk.h | 6 +++--- tree.c | 2 +- unpack-trees.c | 6 +++--- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 7d0779f6cf..2cd06126d2 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -547,7 +547,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec, int old_baselen = base->len; while (tree_entry(tree, &entry)) { - int te_len = tree_entry_len(entry.path, entry.sha1); + int te_len = tree_entry_len(&entry); if (match != 2) { match = tree_entry_interesting(&entry, base, tn_len, pathspec); diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ba3705d1de..b4f7855550 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -979,7 +979,7 @@ static void add_pbase_object(struct tree_desc *tree, while (tree_entry(tree,&entry)) { if (S_ISGITLINK(entry.mode)) continue; - cmp = tree_entry_len(entry.path, entry.sha1) != cmplen ? 1 : + cmp = tree_entry_len(&entry) != cmplen ? 1 : memcmp(name, entry.path, cmplen); if (cmp > 0) continue; diff --git a/tree-diff.c b/tree-diff.c index b3cc2e4753..6782484489 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -21,8 +21,8 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, sha1 = tree_entry_extract(t1, &path1, &mode1); sha2 = tree_entry_extract(t2, &path2, &mode2); - pathlen1 = tree_entry_len(path1, sha1); - pathlen2 = tree_entry_len(path2, sha2); + pathlen1 = tree_entry_len(&t1->entry); + pathlen2 = tree_entry_len(&t2->entry); cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2); if (cmp < 0) { show_entry(opt, "-", t1, base); @@ -85,7 +85,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, unsigned mode; const char *path; const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode); - int pathlen = tree_entry_len(path, sha1); + int pathlen = tree_entry_len(&desc->entry); int old_baselen = base->len; strbuf_add(base, path, pathlen); diff --git a/tree-walk.c b/tree-walk.c index 418107ec83..f5d19f9cc1 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -116,7 +116,7 @@ void setup_traverse_info(struct traverse_info *info, const char *base) char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n) { - int len = tree_entry_len(n->path, n->sha1); + int len = tree_entry_len(n); int pathlen = info->pathlen; path[pathlen + len] = 0; @@ -126,7 +126,7 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str break; path[--pathlen] = '/'; n = &info->name; - len = tree_entry_len(n->path, n->sha1); + len = tree_entry_len(n); info = info->prev; pathlen -= len; } @@ -253,7 +253,7 @@ static void extended_entry_extract(struct tree_desc_x *t, * The caller wants "first" from this tree, or nothing. */ path = a->path; - len = tree_entry_len(a->path, a->sha1); + len = tree_entry_len(a); switch (check_entry_match(first, first_len, path, len)) { case -1: entry_clear(a); @@ -271,7 +271,7 @@ static void extended_entry_extract(struct tree_desc_x *t, while (probe.size) { entry_extract(&probe, a); path = a->path; - len = tree_entry_len(a->path, a->sha1); + len = tree_entry_len(a); switch (check_entry_match(first, first_len, path, len)) { case -1: entry_clear(a); @@ -362,7 +362,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info) e = entry + i; if (!e->path) continue; - len = tree_entry_len(e->path, e->sha1); + len = tree_entry_len(e); if (!first) { first = e->path; first_len = len; @@ -381,7 +381,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info) /* Cull the ones that are not the earliest */ if (!e->path) continue; - len = tree_entry_len(e->path, e->sha1); + len = tree_entry_len(e); if (name_compare(e->path, len, first, first_len)) entry_clear(e); } @@ -434,8 +434,8 @@ static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char int entrylen, cmp; sha1 = tree_entry_extract(t, &entry, mode); + entrylen = tree_entry_len(&t->entry); update_tree_entry(t); - entrylen = tree_entry_len(entry, sha1); if (entrylen > namelen) continue; cmp = memcmp(name, entry, entrylen); @@ -596,7 +596,7 @@ int tree_entry_interesting(const struct name_entry *entry, ps->max_depth); } - pathlen = tree_entry_len(entry->path, entry->sha1); + pathlen = tree_entry_len(entry); for (i = ps->nr - 1; i >= 0; i--) { const struct pathspec_item *item = ps->items+i; diff --git a/tree-walk.h b/tree-walk.h index 0089581e1d..884d01ac33 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -20,9 +20,9 @@ static inline const unsigned char *tree_entry_extract(struct tree_desc *desc, co return desc->entry.sha1; } -static inline int tree_entry_len(const char *name, const unsigned char *sha1) +static inline int tree_entry_len(const struct name_entry *ne) { - return (const char *)sha1 - name - 1; + return (const char *)ne->sha1 - ne->path - 1; } void update_tree_entry(struct tree_desc *); @@ -58,7 +58,7 @@ extern void setup_traverse_info(struct traverse_info *info, const char *base); static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n) { - return info->pathlen + tree_entry_len(n->path, n->sha1); + return info->pathlen + tree_entry_len(n); } extern int tree_entry_interesting(const struct name_entry *, struct strbuf *, int, const struct pathspec *ps); diff --git a/tree.c b/tree.c index 698ecf7af1..e62219803b 100644 --- a/tree.c +++ b/tree.c @@ -99,7 +99,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base, else continue; - len = tree_entry_len(entry.path, entry.sha1); + len = tree_entry_len(&entry); strbuf_add(base, entry.path, len); strbuf_addch(base, '/'); retval = read_tree_1(lookup_tree(sha1), diff --git a/unpack-trees.c b/unpack-trees.c index 8282f5e5f6..7c9ecf665d 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -446,7 +446,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask, newinfo.prev = info; newinfo.pathspec = info->pathspec; newinfo.name = *p; - newinfo.pathlen += tree_entry_len(p->path, p->sha1) + 1; + newinfo.pathlen += tree_entry_len(p) + 1; newinfo.conflicts |= df_conflicts; for (i = 0; i < n; i++, dirmask >>= 1) { @@ -495,7 +495,7 @@ static int do_compare_entry(const struct cache_entry *ce, const struct traverse_ ce_len -= pathlen; ce_name = ce->name + pathlen; - len = tree_entry_len(n->path, n->sha1); + len = tree_entry_len(n); return df_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode); } @@ -626,7 +626,7 @@ static int find_cache_pos(struct traverse_info *info, struct unpack_trees_options *o = info->data; struct index_state *index = o->src_index; int pfxlen = info->pathlen; - int p_len = tree_entry_len(p->path, p->sha1); + int p_len = tree_entry_len(p); for (pos = o->cache_bottom; pos < index->cache_nr; pos++) { struct cache_entry *ce = index->cache[pos]; -- cgit v1.2.3 From 5fb8c05f2e5da483898019f7abc0e52e07bc0c50 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 27 Oct 2011 11:18:40 -0700 Subject: get_tree_entry(): do not call find_tree_entry() on an empty tree We know we will find nothing. This incidentally squelches false warning from gcc about potentially uninitialized usage of t.entry fields. For an empty tree, it is true that init_tree_desc() does not call decode_tree_entry() and the tree_desc is left uninitialized, but find_tree_entry() only calls tree_entry_extract() that uses the tree_desc while it has more things to read from the tree, so the uninitialized t.entry fields are never used in such a case anyway. Signed-off-by: Junio C Hamano --- tree-walk.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tree-walk.c b/tree-walk.c index f5d19f9cc1..cc2f14a0d6 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -465,7 +465,6 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch int retval; void *tree; unsigned long size; - struct tree_desc t; unsigned char root[20]; tree = read_object_with_reference(tree_sha1, tree_type, &size, root); @@ -478,8 +477,13 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch return 0; } - init_tree_desc(&t, tree, size); - retval = find_tree_entry(&t, name, sha1, mode); + if (!size) { + retval = -1; + } else { + struct tree_desc t; + init_tree_desc(&t, tree, size); + retval = find_tree_entry(&t, name, sha1, mode); + } free(tree); return retval; } -- cgit v1.2.3 From 02cb67530e62e77d437cd68b4bb4307ab752b6a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2011 17:36:11 +1100 Subject: read_directory_recursive: reduce one indentation level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- dir.c | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/dir.c b/dir.c index 6c0d782579..0a78d00b54 100644 --- a/dir.c +++ b/dir.c @@ -968,34 +968,34 @@ static int read_directory_recursive(struct dir_struct *dir, { DIR *fdir = opendir(*base ? base : "."); int contents = 0; + struct dirent *de; + char path[PATH_MAX + 1]; - if (fdir) { - struct dirent *de; - char path[PATH_MAX + 1]; - memcpy(path, base, baselen); - - while ((de = readdir(fdir)) != NULL) { - int len; - switch (treat_path(dir, de, path, sizeof(path), - baselen, simplify, &len)) { - case path_recurse: - contents += read_directory_recursive - (dir, path, len, 0, simplify); - continue; - case path_ignored: - continue; - case path_handled: - break; - } - contents++; - if (check_only) - goto exit_early; - else - dir_add_name(dir, path, len); + if (!fdir) + return 0; + + memcpy(path, base, baselen); + + while ((de = readdir(fdir)) != NULL) { + int len; + switch (treat_path(dir, de, path, sizeof(path), + baselen, simplify, &len)) { + case path_recurse: + contents += read_directory_recursive(dir, path, len, 0, simplify); + continue; + case path_ignored: + continue; + case path_handled: + break; } -exit_early: - closedir(fdir); + contents++; + if (check_only) + goto exit_early; + else + dir_add_name(dir, path, len); } +exit_early: + closedir(fdir); return contents; } -- cgit v1.2.3 From d688cf07b15b664a2164c3d92bcb5e8265400a2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2011 17:36:10 +1100 Subject: tree_entry_interesting(): give meaningful names to return values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is a basic code hygiene to avoid magic constants that are unnamed. Besides, this helps extending the value later on for "interesting, but cannot decide if the entry truely matches yet" (ie. prefix matches) Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/grep.c | 9 +++++---- list-objects.c | 9 +++++---- tree-diff.c | 16 +++++++++------- tree-walk.c | 45 +++++++++++++++++++++------------------------ tree-walk.h | 12 +++++++++++- tree.c | 9 +++++---- 6 files changed, 56 insertions(+), 44 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 2cd06126d2..2fc51fa2dc 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -542,18 +542,19 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec, struct tree_desc *tree, struct strbuf *base, int tn_len) { - int hit = 0, match = 0; + int hit = 0; + enum interesting match = entry_not_interesting; struct name_entry entry; int old_baselen = base->len; while (tree_entry(tree, &entry)) { int te_len = tree_entry_len(&entry); - if (match != 2) { + if (match != all_entries_interesting) { match = tree_entry_interesting(&entry, base, tn_len, pathspec); - if (match < 0) + if (match == all_entries_not_interesting) break; - if (match == 0) + if (match == entry_not_interesting) continue; } diff --git a/list-objects.c b/list-objects.c index 39d80c0175..3dd4a96019 100644 --- a/list-objects.c +++ b/list-objects.c @@ -71,7 +71,8 @@ static void process_tree(struct rev_info *revs, struct tree_desc desc; struct name_entry entry; struct name_path me; - int match = revs->diffopt.pathspec.nr == 0 ? 2 : 0; + enum interesting match = revs->diffopt.pathspec.nr == 0 ? + all_entries_interesting: entry_not_interesting; int baselen = base->len; if (!revs->tree_objects) @@ -97,12 +98,12 @@ static void process_tree(struct rev_info *revs, init_tree_desc(&desc, tree->buffer, tree->size); while (tree_entry(&desc, &entry)) { - if (match != 2) { + if (match != all_entries_interesting) { match = tree_entry_interesting(&entry, base, 0, &revs->diffopt.pathspec); - if (match < 0) + if (match == all_entries_not_interesting) break; - if (match == 0) + if (match == entry_not_interesting) continue; } diff --git a/tree-diff.c b/tree-diff.c index 6782484489..7a51d091b5 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -64,14 +64,14 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, static void show_tree(struct diff_options *opt, const char *prefix, struct tree_desc *desc, struct strbuf *base) { - int match = 0; + enum interesting match = entry_not_interesting; for (; desc->size; update_tree_entry(desc)) { - if (match != 2) { + if (match != all_entries_interesting) { match = tree_entry_interesting(&desc->entry, base, 0, &opt->pathspec); - if (match < 0) + if (match == all_entries_not_interesting) break; - if (match == 0) + if (match == entry_not_interesting) continue; } show_entry(opt, prefix, desc, base); @@ -114,12 +114,13 @@ static void show_entry(struct diff_options *opt, const char *prefix, } static void skip_uninteresting(struct tree_desc *t, struct strbuf *base, - struct diff_options *opt, int *match) + struct diff_options *opt, + enum interesting *match) { while (t->size) { *match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec); if (*match) { - if (*match < 0) + if (*match == all_entries_not_interesting) t->size = 0; break; } @@ -132,7 +133,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, { struct strbuf base; int baselen = strlen(base_str); - int t1_match = 0, t2_match = 0; + enum interesting t1_match = entry_not_interesting; + enum interesting t2_match = entry_not_interesting; /* Enable recursion indefinitely */ opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE); diff --git a/tree-walk.c b/tree-walk.c index cc2f14a0d6..4a03e9bd21 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -577,27 +577,23 @@ static int match_dir_prefix(const char *base, * * Pre-condition: either baselen == base_offset (i.e. empty path) * or base[baselen-1] == '/' (i.e. with trailing slash). - * - * Return: - * - 2 for "yes, and all subsequent entries will be" - * - 1 for yes - * - zero for no - * - negative for "no, and no subsequent entries will be either" */ -int tree_entry_interesting(const struct name_entry *entry, - struct strbuf *base, int base_offset, - const struct pathspec *ps) +enum interesting tree_entry_interesting(const struct name_entry *entry, + struct strbuf *base, int base_offset, + const struct pathspec *ps) { int i; int pathlen, baselen = base->len - base_offset; - int never_interesting = ps->has_wildcard ? 0 : -1; + int never_interesting = ps->has_wildcard ? + entry_not_interesting : all_entries_not_interesting; if (!ps->nr) { if (!ps->recursive || ps->max_depth == -1) - return 2; - return !!within_depth(base->buf + base_offset, baselen, - !!S_ISDIR(entry->mode), - ps->max_depth); + return all_entries_interesting; + return within_depth(base->buf + base_offset, baselen, + !!S_ISDIR(entry->mode), + ps->max_depth) ? + entry_interesting : entry_not_interesting; } pathlen = tree_entry_len(entry); @@ -614,12 +610,13 @@ int tree_entry_interesting(const struct name_entry *entry, goto match_wildcards; if (!ps->recursive || ps->max_depth == -1) - return 2; + return all_entries_interesting; - return !!within_depth(base_str + matchlen + 1, - baselen - matchlen - 1, - !!S_ISDIR(entry->mode), - ps->max_depth); + return within_depth(base_str + matchlen + 1, + baselen - matchlen - 1, + !!S_ISDIR(entry->mode), + ps->max_depth) ? + entry_interesting : entry_not_interesting; } /* Either there must be no base, or the base must match. */ @@ -627,18 +624,18 @@ int tree_entry_interesting(const struct name_entry *entry, if (match_entry(entry, pathlen, match + baselen, matchlen - baselen, &never_interesting)) - return 1; + return entry_interesting; if (ps->items[i].use_wildcard) { if (!fnmatch(match + baselen, entry->path, 0)) - return 1; + return entry_interesting; /* * Match all directories. We'll try to * match files later on. */ if (ps->recursive && S_ISDIR(entry->mode)) - return 1; + return entry_interesting; } continue; @@ -657,7 +654,7 @@ match_wildcards: if (!fnmatch(match, base->buf + base_offset, 0)) { strbuf_setlen(base, base_offset + baselen); - return 1; + return entry_interesting; } strbuf_setlen(base, base_offset + baselen); @@ -666,7 +663,7 @@ match_wildcards: * later on. */ if (ps->recursive && S_ISDIR(entry->mode)) - return 1; + return entry_interesting; } return never_interesting; /* No matches */ } diff --git a/tree-walk.h b/tree-walk.h index 884d01ac33..2bf0db9814 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -61,6 +61,16 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru return info->pathlen + tree_entry_len(n); } -extern int tree_entry_interesting(const struct name_entry *, struct strbuf *, int, const struct pathspec *ps); +/* in general, positive means "kind of interesting" */ +enum interesting { + all_entries_not_interesting = -1, /* no, and no subsequent entries will be either */ + entry_not_interesting = 0, + entry_interesting = 1, + all_entries_interesting = 2 /* yes, and all subsequent entries will be */ +}; + +extern enum interesting tree_entry_interesting(const struct name_entry *, + struct strbuf *, int, + const struct pathspec *ps); #endif diff --git a/tree.c b/tree.c index e62219803b..676e9f710c 100644 --- a/tree.c +++ b/tree.c @@ -52,7 +52,8 @@ static int read_tree_1(struct tree *tree, struct strbuf *base, struct tree_desc desc; struct name_entry entry; unsigned char sha1[20]; - int len, retval = 0, oldlen = base->len; + int len, oldlen = base->len; + enum interesting retval = entry_not_interesting; if (parse_tree(tree)) return -1; @@ -60,11 +61,11 @@ static int read_tree_1(struct tree *tree, struct strbuf *base, init_tree_desc(&desc, tree->buffer, tree->size); while (tree_entry(&desc, &entry)) { - if (retval != 2) { + if (retval != all_entries_interesting) { retval = tree_entry_interesting(&entry, base, 0, pathspec); - if (retval < 0) + if (retval == all_entries_not_interesting) break; - if (retval == 0) + if (retval == entry_not_interesting) continue; } -- cgit v1.2.3 From 9c6bebd14233af291ffb8dde2975b06c76e9ce5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 24 Oct 2011 17:36:12 +1100 Subject: tree_entry_interesting: make use of local pointer "item" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- tree-walk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree-walk.c b/tree-walk.c index 4a03e9bd21..f82dba6a1f 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -626,7 +626,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry, &never_interesting)) return entry_interesting; - if (ps->items[i].use_wildcard) { + if (item->use_wildcard) { if (!fnmatch(match + baselen, entry->path, 0)) return entry_interesting; @@ -642,7 +642,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry, } match_wildcards: - if (!ps->items[i].use_wildcard) + if (!item->use_wildcard) continue; /* -- cgit v1.2.3 From ea4f9685cb432caf357ad8af56a91877b6d04663 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 27 Oct 2011 11:42:57 -0700 Subject: unpack_object_header_buffer(): clear the size field upon error The callers do not use the returned size when the function says it did not use any bytes and sets the type to OBJ_BAD, so this should not matter in practice, but it is a good code hygiene anyway. Signed-off-by: Junio C Hamano --- sha1_file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 27f3b9b278..833f5b9b58 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1267,7 +1267,8 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf, while (c & 0x80) { if (len <= used || bitsizeof(long) <= shift) { error("bad object header"); - return 0; + size = used = 0; + break; } c = buf[used++]; size += (c & 0x7f) << shift; -- cgit v1.2.3