Re: [PATCH v5] hfs: initialize B-tree inodes and validate catalog CNIDs

[yangguikun]

Hi Slava,

Thanks for the detailed review.

On set_nlink(): I removed the explicit call, but please note that it
was a no-op: inode_init_always_gfp() sets __i_nlink = 1 (fs/inode.c:238)
for every new inode, and the original iget_locked() path in
hfs_btree_open() never cleared it. So the B-tree inodes have had
i_nlink == 1 since before this series, inode_generic_drop() returns
false for them, and the final iput() in hfs_btree_close() already took
the !drop branch with writeback. I could not construct the
still-dirty-at-put_super scenario outside of an error in the umount
sync; if you have a concrete case in mind, I am happy to test it. The
redundant set_nlink() is still worth removing for clarity.

tz_secondswest is only consumed by hfs_revalidate_dentry(), which runs
from dentries; the B-tree inodes never get one, so I removed that
initialization as well.

I will use the WARN_ON_ONCE() form for the already-cached inode case.

On hfs_iget(): agreed on both counts. Returning ERR_PTR() lets the
corruption cases (-EIO) and allocation failure (-ENOMEM) stay
distinguishable, and both callers currently guess: hfs_lookup()
translates NULL to -EACCES and hfs_fill_super() reports -EINVAL.
minix_iget() already works this way. In v5 I only did what the v4
review suggested (-EIO), but the complete fix is the ERR_PTR()
conversion, so I reverted the dir.c change and will send it as a
separate patch with the caller updates.

Patch v6 follows.

Thanks,
yangguikun