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

Yep. You can commit it or I can?
(You can put reviewed by me if you choose to commit it.)

Thanks, rick

On Thu, Jul 10, 2025 at 5:27 AM Dag-Erling Smørgrav <des@freebsd.org> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If in doubt, forward suspicious emails to IThelp@uoguelph.ca.
>
> 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