[RFC/PATCH] fsck: do not canonicalize modes in trees we are checking
- From
Jeff King <peff@peff.net>
- Date
- Sep 23, 2014, 15:47 UTC
- Message-ID
- <20140923154751.GA19319@peff.net>
Doing so means that we do not actually get to see bogus modes; they are converted into one of our known-good modes by decode_tree_entry. We want to see the raw data so that we can complain about it.
Signed-off-by: Jeff King <peff@peff.net> --- As far as I can tell, fsck's mode-checking has been totally broken basically forever. Which makes me a little nervous to fix it. :) linux.git does have some bogus modes, but they are 100664, which is specifically ignored here unless "fsck --strict" is in effect.
The implementation feels a bit hacky. The global is ugly, but it avoids having to pass options all through the call chain of init_tree_entry, update_tree_entry, etc.
Another option would be to have decode_tree_entry leave the raw mode in the tree_desc, and only canonicalize it during tree_entry_extract (and teach fsck to look at the raw data, not _extract). That would mean reverting 7146e66 (tree-walk: finally switch over tree descriptors to contain a pre-parsed entry, 2014-02-06). That commit says there's no real benefit at the time, but that future code might benefit. I don't know if that future code ever materialized (Kirill cc'd).
I thought at first that commit might have been responsible for this breakage, but I don't think so. The canonicalization has happened in tree_entry_extract since at least 2006, and we have always called that function in fsck.
fsck.c | 2 ++ t/t1450-fsck.sh | 21 +++++++++++++++++++++ tree-walk.c | 4 +++- tree-walk.h | 3 +++ 4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c index 56156ff..ef79396 100644 --- a/fsck.c +++ b/fsck.c @@ -153,6 +153,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) unsigned o_mode; const char *o_name; + decode_tree_raw = 1; init_tree_desc(&desc, item->buffer, item->size); o_mode = 0; @@ -213,6 +214,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) o_name = name; } + decode_tree_raw = 0; retval = 0; if (has_null_sha1) retval += error_func(&item->object, FSCK_WARN, "contains entries pointing to null sha1"); diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index b52397a..ba40c6f 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -176,6 +176,27 @@ test_expect_success 'malformatted tree object' ' grep "error in tree .*contains duplicate file entries" out ' +# Basically an 8-bit clean version of sed 's/$1/$2/g'. +munge_tree_bytes () { + perl -e ' + binmode(STDIN); binmode(STDOUT); + $_ = do { local $/; <STDIN> }; + s/$ARGV[0]/$ARGV[1]/g; + print + ' "$@" +} + +test_expect_success 'bogus mode in tree' ' + test_when_finished "remove_object \$T" && + T=$( + git cat-file tree HEAD >good && + munge_tree_bytes 100644 123456 <good >bad && + git hash-object -w -t tree bad + ) && + git fsck 2>out && + grep "warning.*contains bad file modes" out +' + test_expect_success 'tag pointing to nonexistent' ' cat >invalid-tag <<-\EOF && object ffffffffffffffffffffffffffffffffffffffff diff --git a/tree-walk.c b/tree-walk.c index 5dd9a71..baca766 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -5,6 +5,8 @@ #include "tree.h" #include "pathspec.h" +int decode_tree_raw; + static const char *get_mode(const char *str, unsigned int *modep) { unsigned char c; @@ -37,7 +39,7 @@ static void decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned /* Initialize the descriptor entry */ desc->entry.path = path; - desc->entry.mode = canon_mode(mode); + desc->entry.mode = decode_tree_raw ? mode : canon_mode(mode); desc->entry.sha1 = (const unsigned char *)(path + len); } diff --git a/tree-walk.h b/tree-walk.h index ae7fb3a..9f78e85 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -25,6 +25,9 @@ static inline int tree_entry_len(const struct name_entry *ne) return (const char *)ne->sha1 - ne->path - 1; } +/* If set, do not canonicalize modes in tree entries. */ +extern int decode_tree_raw; + void update_tree_entry(struct tree_desc *); void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size);
-- 2.1.1.496.g1ba8081