Commits

Anonymous committed 0f2303f

[PATCH] Un unoptimize ls-tree behaviour

ls-tree unconditionally called read_sha1_file() for all paths
even when not needed, which was a mistake introduced by me.
Rectify this by first checking S_ISDIR(mode) and read the tree
contents only when it is a tree and we are recursive. There is
no need to read it in any other cases.

The patch also removes the confusing comment that led to this
incorrect implementation.

Thanks to Peter Baudis for noticing this problem.

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

  • Participants
  • Parent commits eec8c63

Comments (0)

Files changed (1)

 		buffer = sha1 + 20;
 		size -= namelen + 20;
 
-		/* XXX: We do some ugly mode heuristics here.
-		 * It seems not worth it to read each file just to get this
-		 * and the file size. -- pasky@ucw.cz
-		 * ... that is, when we are not recursive -- junkio@cox.net
-		 */
-		eltbuf = (recursive ? read_sha1_file(sha1, elttype, &eltsize) :
-			  NULL);
-		if (! eltbuf) {
-			if (recursive)
-				error("cannot read %s", sha1_to_hex(sha1));
-			type = S_ISDIR(mode) ? "tree" : "blob";
-		}
-		else
-			type = elttype;
-
-		printf("%03o\t%s\t%s\t", mode, type, sha1_to_hex(sha1));
+		printf("%06o\t%s\t%s\t", mode,
+		       S_ISDIR(mode) ? "tree" : "blob",
+		       sha1_to_hex(sha1));
 		print_path_prefix(prefix);
 		fputs(path, stdout);
 		putchar(line_termination);
 
-		if (eltbuf && !strcmp(type, "tree")) {
-			this_prefix.name = path;
-			list_recursive(eltbuf, elttype, eltsize, &this_prefix);
+		if (! recursive || ! S_ISDIR(mode))
+			continue;
+
+		if (! (eltbuf = read_sha1_file(sha1, elttype, &eltsize)) ) {
+			error("cannot read %s", sha1_to_hex(sha1));
+			continue;
 		}
+		this_prefix.name = path;
+		list_recursive(eltbuf, elttype, eltsize, &this_prefix);
 		free(eltbuf);
 	}
 }
 	return 0;
 }
 
-static void _usage(void)
-{
-	usage("ls-tree [-r] [-z] <key>");
-}
+static const char *ls_tree_usage = "ls-tree [-r] [-z] <key>";
 
 int main(int argc, char **argv)
 {
 			recursive = 1;
 			break;
 		default:
-			_usage();
+			usage(ls_tree_usage);
 		}
 		argc--; argv++;
 	}
 
 	if (argc != 2)
-		_usage();
+		usage(ls_tree_usage);
 	if (get_sha1_hex(argv[1], sha1) < 0)
-		_usage();
+		usage(ls_tree_usage);
 	sha1_file_directory = getenv(DB_ENVIRONMENT);
 	if (!sha1_file_directory)
 		sha1_file_directory = DEFAULT_DB_ENVIRONMENT;