Merge branch 'nd/misc-cleanups'

* nd/misc-cleanups:
  unpack_object_header_buffer(): clear the size field upon error
  tree_entry_interesting: make use of local pointer "item"
  tree_entry_interesting(): give meaningful names to return values
  read_directory_recursive: reduce one indentation level
  get_tree_entry(): do not call find_tree_entry() on an empty tree
  tree-walk.c: do not leak internal structure in tree_entry_len()

builtin/grep.c

@@ -557,18 +557,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.path, entry.sha1);
+		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;
 		}
 

builtin/pack-objects.c

@@ -1015,7 +1015,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;

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) {
+	if (!fdir)
-		struct dirent *de;
+		return 0;
-		char path[PATH_MAX + 1];
-		memcpy(path, base, baselen);
 
-		while ((de = readdir(fdir)) != NULL) {
+	memcpy(path, base, baselen);
-			int len;
+
-			switch (treat_path(dir, de, path, sizeof(path),
+	while ((de = readdir(fdir)) != NULL) {
-					   baselen, simplify, &len)) {
+		int len;
-			case path_recurse:
+		switch (treat_path(dir, de, path, sizeof(path),
-				contents += read_directory_recursive
+				   baselen, simplify, &len)) {
-					(dir, path, len, 0, simplify);
+		case path_recurse:
-				continue;
+			contents += read_directory_recursive(dir, path, len, 0, simplify);
-			case path_ignored:
+			continue;
-				continue;
+		case path_ignored:
-			case path_handled:
+			continue;
-				break;
+		case path_handled:
-			}
+			break;
-			contents++;
-			if (check_only)
-				goto exit_early;
-			else
-				dir_add_name(dir, path, len);
 		}
-exit_early:
+		contents++;
-		closedir(fdir);
+		if (check_only)
+			goto exit_early;
+		else
+			dir_add_name(dir, path, len);
 	}
+exit_early:
+	closedir(fdir);
 
 	return contents;
 }

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;
 		}
 

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;

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);
+	pathlen1 = tree_entry_len(&t1->entry);
-	pathlen2 = tree_entry_len(path2, sha2);
+	pathlen2 = tree_entry_len(&t2->entry);
 	cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
 	if (cmp < 0) {
 		show_entry(opt, "-", t1, base);
@@ -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);
@@ -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);
@@ -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);

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);
@@ -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);
+	if (!size) {
-	retval = find_tree_entry(&t, name, sha1, mode);
+		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;
 }
@@ -573,30 +577,26 @@ 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,
+enum interesting tree_entry_interesting(const struct name_entry *entry,
-			   struct strbuf *base, int base_offset,
+					struct strbuf *base, int base_offset,
-			   const struct pathspec *ps)
+					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 all_entries_interesting;
-		return !!within_depth(base->buf + base_offset, baselen,
+		return within_depth(base->buf + base_offset, baselen,
-				      !!S_ISDIR(entry->mode),
+				    !!S_ISDIR(entry->mode),
-				      ps->max_depth);
+				    ps->max_depth) ?
+			entry_interesting : entry_not_interesting;
 	}
 
-	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;
@@ -610,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,
+			return within_depth(base_str + matchlen + 1,
-					      baselen - matchlen - 1,
+					    baselen - matchlen - 1,
-					      !!S_ISDIR(entry->mode),
+					    !!S_ISDIR(entry->mode),
-					      ps->max_depth);
+					    ps->max_depth) ?
+				entry_interesting : entry_not_interesting;
 		}
 
 		/* Either there must be no base, or the base must match. */
@@ -623,25 +624,25 @@ 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 (item->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;
 		}
 
 match_wildcards:
-		if (!ps->items[i].use_wildcard)
+		if (!item->use_wildcard)
 			continue;
 
 		/*
@@ -653,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);
 
@@ -662,7 +663,7 @@ match_wildcards:
 		 * later on.
 		 */
 		if (ps->recursive && S_ISDIR(entry->mode))
-			return 1;
+			return entry_interesting;
 	}
 	return never_interesting; /* No matches */
 }

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,9 +58,19 @@ 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);
+/* 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

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;
 		}
 
@@ -99,7 +100,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),

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];