From a3d6bec5f0e90a5a5b96cccab21fd1bf8fb97f17 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 3 May 2025 17:20:02 -0500 Subject: [PATCH] Fixed a double deorphan caused by relocation mid dir remove Long story short: There is a specific case where removing a directory can trigger a deorphan pass, but lfs_remove did not check for this, would try to clean up the (already cleaned) directory orphan, and trigger an assert: lfs.c:4890:assert: assert failed with false, expected eq true LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0x000 || orphans >= 0); The specific case being a remove commit that triggers a relocation that creates an orphan. This is also possible in lfs_rename, but only if you're renaming a directory that implies a remove, which is a pretty rare operation. --- This was probably an oversight introduced in the non-recursive commit logic rework. Fortunately the fix is to just check if we even have an orphan before trying to remove it. We can rely on this instead of the file type, so this fix shouldn't even increase the code size. Found and root-caused by Hugh-Baoa --- lfs.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lfs.c b/lfs.c index 28a632b6..05b1ca10 100644 --- a/lfs.c +++ b/lfs.c @@ -3932,7 +3932,9 @@ static int lfs_remove_(lfs_t *lfs, const char *path) { } lfs->mlist = dir.next; - if (lfs_tag_type3(tag) == LFS_TYPE_DIR) { + if (lfs_gstate_hasorphans(&lfs->gstate)) { + LFS_ASSERT(lfs_tag_type3(tag) == LFS_TYPE_DIR); + // fix orphan err = lfs_fs_preporphans(lfs, -1); if (err) { @@ -4076,8 +4078,10 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) { } lfs->mlist = prevdir.next; - if (prevtag != LFS_ERR_NOENT - && lfs_tag_type3(prevtag) == LFS_TYPE_DIR) { + if (lfs_gstate_hasorphans(&lfs->gstate)) { + LFS_ASSERT(prevtag != LFS_ERR_NOENT + && lfs_tag_type3(prevtag) == LFS_TYPE_DIR); + // fix orphan err = lfs_fs_preporphans(lfs, -1); if (err) {