Re: git: c5d72d29fe0e - main - nfsv4: Add support for the NFSv4 hidden and system attributes

Rick Macklem <rmacklem@FreeBSD.org> writes:
> diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
> index 0049d7edca33..fbfcdafaa06b 100644
> --- a/sys/fs/nfsclient/nfs_clvnops.c
> +++ b/sys/fs/nfsclient/nfs_clvnops.c
> [...]
> @@ -1092,7 +1100,8 @@ nfs_setattr(struct vop_setattr_args *ap)
>  	    vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL ||
>  	    vap->va_mtime.tv_sec != VNOVAL ||
>  	    vap->va_birthtime.tv_sec != VNOVAL ||
> -	    vap->va_mode != (mode_t)VNOVAL) &&
> +	    vap->va_mode != (mode_t)VNOVAL ||
> +	    vap->va_flags != (u_long)VNOVAL) &&
>  	    (vp->v_mount->mnt_flag & MNT_RDONLY))
>  		return (EROFS);
>  	if (vap->va_size != VNOVAL) {

vap->va_flags was already checked (in the line just before the first
line of context), albeit without a cast.  Coverity erroneously claims
that this causes the entire expression to always be true, because it
thinks VNOVAL and (u_long)VNOVAL are two different values.  That's not
the case, but you probably still want this:

diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
index fbfcdafaa06b..fa451887e73e 100644
--- a/sys/fs/nfsclient/nfs_clvnops.c
+++ b/sys/fs/nfsclient/nfs_clvnops.c
@@ -1096,12 +1096,11 @@ nfs_setattr(struct vop_setattr_args *ap)
 	/*
 	 * Disallow write attempts if the filesystem is mounted read-only.
 	 */
-  	if ((vap->va_flags != VNOVAL || vap->va_uid != (uid_t)VNOVAL ||
+	if ((vap->va_flags != (u_long)VNOVAL || vap->va_uid != (uid_t)VNOVAL ||
 	    vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL ||
 	    vap->va_mtime.tv_sec != VNOVAL ||
 	    vap->va_birthtime.tv_sec != VNOVAL ||
-	    vap->va_mode != (mode_t)VNOVAL ||
-	    vap->va_flags != (u_long)VNOVAL) &&
+	    vap->va_mode != (mode_t)VNOVAL) &&
 	    (vp->v_mount->mnt_flag & MNT_RDONLY))
 		return (EROFS);
 	if (vap->va_size != VNOVAL) {

DES
-- 
Dag-Erling Smørgrav - des@FreeBSD.org