birthtime initialization
Bruce Evans
brde at optusnet.com.au
Tue Sep 9 18:46:14 UTC 2008
On Wed, 3 Sep 2008, Jaakko Heinonen wrote:
> I further updated the patch.
>
> On 2008-07-25, Bruce Evans wrote:
>> On Fri, 25 Jul 2008, Jaakko Heinonen wrote:
>>> On 2008-07-24, Bruce Evans wrote:
>>>> First, the fields shouldn't be initialized using VATTR_NULL() in
>>>> VOP_GETATTR().
>
> I removed VATTR_NULL() from xfs, mqueuefs, pseudofs, tmpfs, devfs
> fdescfs and vattr_null() from devfs and portalfs. I also removed zeroing
> from nfs.
>
>>> What do you think that is a proper default value for va_rdev? Some file
>>> systems set it to 0 and some to VNOVAL.
>>
>> Either NODEV or VNOVAL explicitly translated late to NODEV.
>
> I changed following file systems to initialize va_rdev to NODEV instead
> of 0 or VNOVAL (when appropriate): mqueuefs, tmpfs, portalfs, smbfs,
> ntfs, fdescfs and msdosfs. Also in vn_stat() va_rdev is now initialized
> to VNOVAL and explicitly translated to NODEV after the VOP_GETATTR()
> call.
>
> I have tested the patch with these file systems: UFS2, UFS1, ext2fs,
> ntfs, cd9660, udf, procfs, devfs, xfs, reiserfs, fdescfs, msdosfs,
> mqueuefs, nfs, smbfs, portalfs.
I like this version, but didn't check many details.
> Index: sys/kern/vfs_vnops.c
> ===================================================================
> --- sys/kern/vfs_vnops.c (revision 182592)
> +++ sys/kern/vfs_vnops.c (working copy)
> @@ -703,6 +703,17 @@ vn_stat(vp, sb, active_cred, file_cred,
> #endif
>
> vap = &vattr;
> +
> + /*
> + * Initialize defaults for new and unusual fields, so that file
> + * systems which don't support these fields don't need to know
> + * about them.
> + */
> + vap->va_birthtime.tv_sec = -1;
> + vap->va_birthtime.tv_nsec = 0;
> + vap->va_fsid = VNOVAL;
> + vap->va_rdev = VNOVAL;
Shouldn't this be NODEV? NODEV is ((dev_t)-1) and VNOVAL is plain (-1),
so their values are identical after assignment to va_rdev...
> +
> error = VOP_GETATTR(vp, vap, active_cred);
> if (error)
> return (error);
> @@ -750,7 +761,10 @@ vn_stat(vp, sb, active_cred, file_cred,
> sb->st_nlink = vap->va_nlink;
> sb->st_uid = vap->va_uid;
> sb->st_gid = vap->va_gid;
> - sb->st_rdev = vap->va_rdev;
> + if (vap->va_rdev == VNOVAL)
> + sb->st_rdev = NODEV;
> + else
> + sb->st_rdev = vap->va_rdev;
... this change seems to have no effect on machines with 32-bit 2's complement
ints and to be wrong on other machines. Assignment of VNOVAL to va_rdev
changes its value from -1 to 0xFFFFFFFFU, so it can only compare equal
to VNOVAL if int has the same size as dev_t or there is stronger magic.
When the comparision is fixed to compare with the assigned value
(dev_t)VNOVAL == (dev_t)(-1) == NODEV, it is clear that the change has
no effect.
Bruce
More information about the freebsd-fs
mailing list