[PATCH] diff: mode bits fixes

[Junio C Hamano <junkio@cox.net>]

The core GIT repository has trees that record regular file mode
in 0664 instead of normalized 0644 pattern.  Comparing such a
tree with another tree that records the same file in 0644
pattern without content changes with git-diff-tree causes it to
feed otherwise unmodified pairs to the diff_change() routine,
which triggers a sanity check routine and barfs.  This patch
fixes the problem, along with the fix to another caller that
uses unnormalized mode bits to call diff_change() routine in a
similar way.

Without this patch, you will see "fatal error" from diff-tree
when you run git-deltafy-script on the core GIT repository
itself.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

*** Linus, I decided to bite the bullet and audited the callers.
*** There was only one that was not quite right, other than the
*** diff-tree one.  Please disregard the one I sent last night
*** which incorrectly said "S_ISDIR() || S_ISREG()".  Please
*** also disregard the other one that silently ignores
*** unmodified filepairs in the output routine.  Warning about
*** callers should be the most appropriate action as this
*** version does.

 diff.h       |    4 ++++
 diffcore.h   |    4 ----
 diff-files.c |    8 +++-----
 diff-tree.c  |    4 +++-
 diff.c       |   12 +++++++-----
 5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -4,6 +4,10 @@
 #ifndef DIFF_H
 #define DIFF_H
 
+#define DIFF_FILE_CANON_MODE(mode) \
+	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
+	S_ISLNK(mode) ? S_IFLNK : S_IFDIR)
+
 extern void diff_addremove(int addremove,
 			   unsigned mode,
 			   const unsigned char *sha1,
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -59,10 +59,6 @@ struct diff_filepair {
 
 #define DIFF_PAIR_MODE_CHANGED(p) ((p)->one->mode != (p)->two->mode)
 
-#define DIFF_FILE_CANON_MODE(mode) \
-	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
-	S_ISLNK(mode) ? S_IFLNK : S_IFDIR)
-
 extern void diff_free_filepair(struct diff_filepair *);
 
 extern int diff_unmodified_pair(struct diff_filepair *);
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -88,7 +88,7 @@ int main(int argc, const char **argv)
 
 	for (i = 0; i < entries; i++) {
 		struct stat st;
-		unsigned int oldmode, mode;
+		unsigned int oldmode;
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 
@@ -116,10 +116,8 @@ int main(int argc, const char **argv)
 			continue;
 
 		oldmode = ntohl(ce->ce_mode);
-		mode = (S_ISLNK(st.st_mode) ? S_IFLNK :
-			S_IFREG | ce_permissions(st.st_mode));
-
-		show_modified(oldmode, mode, ce->sha1, null_sha1,
+		show_modified(oldmode, DIFF_FILE_CANON_MODE(st.st_mode),
+			      ce->sha1, null_sha1,
 			      ce->name);
 	}
 	diffcore_std((1 < argc) ? argv + 1 : NULL,
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -44,10 +44,12 @@ static const unsigned char *extract(void
 	int len = strlen(tree)+1;
 	const unsigned char *sha1 = tree + len;
 	const char *path = strchr(tree, ' ');
+	unsigned int mode;
 
-	if (!path || size < len + 20 || sscanf(tree, "%o", modep) != 1)
+	if (!path || size < len + 20 || sscanf(tree, "%o", &mode) != 1)
 		die("corrupt tree file");
 	*pathp = path+1;
+	*modep = DIFF_FILE_CANON_MODE(mode);
 	return sha1;
 }
 
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -854,12 +854,14 @@ static void diff_resolve_rename_copy(voi
 		else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
 			 p->one->mode != p->two->mode)
 			p->status = 'M';
-		else
-			/* this is a "no-change" entry.
-			 * should not happen anymore.
-			 * p->status = 'X';
+		else {
+			/* This is a "no-change" entry and should not
+			 * happen anymore, but prepare for broken callers.
 			 */
-			die("internal error in diffcore: unmodified entry remains");
+			error("feeding unmodified %s to diffcore",
+			      p->one->path);
+			p->status = 'X';
+		}
 	}
 	diff_debug_queue("resolve-rename-copy done", q);
 }
------------