{"thread":{"id":"60039","subject":"fsck: various diagnostics never triggering","startedAt":"2023-07-29T16:18:02Z","lastAt":"2023-07-29T16:18:02Z","messageCount":1,"participants":["Xavier Morel"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"479980","messageId":"305BF2BA-6F33-4C34-8E2F-DB194AC38C2C@masklinn.net","threadId":"60039","inReplyTo":null,"subject":"fsck: various diagnostics never triggering","fromName":"Xavier Morel","fromEmail":"xavier.morel@masklinn.net","sentAt":"2023-07-29T16:17:48Z","receivedAt":"2023-07-29T16:18:02Z","isPatch":false,"sender":{"key":"xavier.morel@masklinn.net","avatar":null},"body":"First I'd like to thank John Cai for the work of collating the fsck\nmsg-id, figuring out what to look for is much easier when it's all in\none place.\n\nWhile looking at git's handling of invalid objects (again) I found out\nthat several documented fsck diagnostics can not fire — and\nfurthermore have somewhat debatable diag levels — because they trigger\na parsing error upstream from the fsck code which actually checks the\nrelevant properties.\n\nSome of them are actually present in t/t1451-fsck-buffer.sh but I'm\nnot sure if that is relevant in any way.\n\nThe testing / checking was done with Git 2.41.\n\n------\n\n    emptyName (WARN) A path contains an empty name.\n\n`fsck_tree` ultimately calls `decode_tree_entry` which fails (returns\n-1) if the mode is invalid (e.g. the mode is immediately followed by\n\\0) or the path is empty. In fsck_tree, this translates to a generic\n`badTree` error.\n\nAlso not sure how relevant that is, but a badTree error yields a lot\nof spam e.g. when specifically fsck-ing the broken tree I get:\n\n    error: empty filename in tree entry\n    error in tree {oid}: broken links\n    error: empty filename in tree entry\n    error in tree {oid}: badTree: cannot be parsed as a tree\n    error: empty filename in tree entry\n\nbut for the specific error message, this reproduces for every\nerror which results in a \"badTree\" (mode is empty, mode is\ninvalid, oid is too short, oid is too long, tree object is too\nshort).\n\nIt's possible that part of it is an artefact of my test setup e.g. I\nget this if I fsck the object specifically (passing its oid to \"git\nfsck\"), asking fsck to check the repository rather than the specific\nobject seems to remove the last message in the sequence.\n\n    missingTree (ERROR) Missing tree line in a commit object.\n\nIf a commit has no `tree` line, or a `tree` line which is not 45 bytes\nlong (excluding newline), fsck reports\n\n     error: bogus commit object {oid}\n     error: {oid}: object could not be parsed: {path}\n     error: {oid}: object missing\n\nbut no missingTree.\n\n    badTreeSha1 (ERROR) A tree has an invalid format.\n\nIf a commit has a tree line which is 45 bytes long, but the last 40\nbytes are not a valid hex string, fsck reports\n\n     error: bad tree pointer in commit {oid}\n     error: {oid}: object could not be parsed: {path}\n     error: {oid}: object missing\n\n    badParentSha1 (ERROR) A commit object has a bad parent sha1.\n\nHere whether the parent line is too short, too long, or not hex, fsck reports\n\n    error: bad parents in commit {oid}\n    error: {oid}: object could not be parsed: {oid}\n    error: {oid}: object missing\n\n-------\n\nFinally at least one diagnostic seems to have an incomplete documentation:\n\n    fullPathname (WARN) A path contains the full path starting with \"/\"\n\nbut the code uses \"strchr\" on the name, so it checks if the name\ncontains a \"/\" anywhere. The message is also not the best:\n\n> contains full pathnames\n\n-------\n\nThere is also a minor inconsistency in the authorship date parsing:\n\n    0+0000\n\ntriggers \"zeroPaddedDate\", but \n\n    1+0000\n\ntriggers \"badDate\".\n\nI assume that is because the code first checks if the date is a 0\nfollowed by a non-space (assuming it's zero-padded), and only after\nthat does it check if the timestamp is followed by a space.\n\nThis is probably too minor an issue to bother complexifying the checker\nthough."}]}