Suspect code in fsck_ext2fs

[Antoine LECA <antoine.leca.1%gmail.com@localhost>]

Hi guys,

Hope this is the best list for that subject; do not doubt to redirect me
to a better place though.

While I am trying to chase a bug in my (non-NBSD) ext2fs code, I found
what I suspect to be a problem in fsck_ext2fs, as the attached patch
(1st) shows. Unfortunately due my own bug (that I did not solve) I am
not able to confirm there needs to be fixed or not.

My understanding is that in that function dircheck(), returning (1)
means all_is_good, while (0) indicates some problem; also, I understand
a non-zero e2d_type in some entry while either using Rev.0 or with the
INCOMPAT_FTYPE unset, is NOT a normal condition.

While here, I realized that the actual value for e2d_type was not
checked in the case of Rev.1 FS, so I designed another patch (2nd) to
add that check. No doubt it can be better written!

Perhaps related is that, when the function returns(0) while there are
dirty values within the directory entries, then fsck stops on that
directory and proposes to "Salvage" the directory, which I read to be a
pretty drastic operation, dropping all the entries after the faulty one
if I understand correctly (I am all of a newbie to FFS and derivatives,
so do not hesitate to correct me where I am wrong.)


Antoine

commit 0c9092689f4cc3e67964c48bedfe5a57e61c3f3c
Author: Antoine Leca <Antoine.Leca.1%gmail.com@localhost>
Date:   Wed Mar 6 12:11:38 2013 +0000

    fsck_ext2: fix e2d_type (?)

diff --git a/sbin/fsck_ext2fs/dir.c b/sbin/fsck_ext2fs/dir.c
index 8036f52..66d36ec 100644
--- a/sbin/fsck_ext2fs/dir.c
+++ b/sbin/fsck_ext2fs/dir.c
@@ -274,7 +274,7 @@ dircheck(struct inodesc *idesc, struct ext2fs_direct *dp)
        if (sblock.e2fs.e2fs_rev < E2FS_REV1 ||
            (sblock.e2fs.e2fs_features_incompat & EXT2F_INCOMPAT_FTYPE) == 0)
                if (dp->e2d_type != 0)
-                       return (1);
+                       return (0);
        size = EXT2FS_DIRSIZ(dp->e2d_namlen);
        if (reclen < size ||
            idesc->id_filesize < size /* ||