> +static void nfs4_file_get_access(struct nfs4_file *fp, u32 access)
> {
> + int oflag = nfs4_access_to_omode(access);
> +
> + /* Note: relies on NFS4_SHARE_ACCESS_BOTH == READ|WRITE */
> + access &= NFS4_SHARE_ACCESS_BOTH;
> + if (access == 0)
> + return;
> +
> if (oflag == O_RDWR) {
This fragment looks odd to me in several ways.
For one NFS4_SHARE_ACCESS_BOTH isn't == READ|WRITE, although reading it
again I suspect this supposed to mean
NFS4_SHARE_ACCESS_READ|NFS4_SHARE_ACCESS_WRITE. Second why to the &=
on access if it's not used except for the test, or for that matter
why don't we do the check on the oflag?
I can see two sensible ways to do this:
a)
static void nfs4_file_get_access(struct nfs4_file *fp, u32 access)
{
int oflag = nfs4_access_to_omode(access);
if (oflag == O_RDWR) {
__nfs4_file_get_access(fp, O_RDONLY);
__nfs4_file_get_access(fp, O_WRONLY);
} else if (oflag == O_RDONLY || oflag == O_RDONLY)
__nfs4_file_get_access(fp, oflag);
}
}
Or even better just avoid the nfs4_file_get_access call altogether:
static void nfs4_file_get_access(struct nfs4_file *fp, u32 access)
{
WARN_ON_ONCE(access & ~NFS4_SHARE_ACCESS_BOTH);
if (access & NFS4_SHARE_ACCESS_WRITE)
__nfs4_file_get_access(fp, O_WRONLY);
if (access & NFS4_SHARE_ACCESS_READ)
__nfs4_file_get_access(fp, O_RDONLY);
}
Same for the put side.
Btw, what is the story about the third fd in fi_fds? Seems like
nfs4_get_vfs_file can put a file pointer in there, but
nfs4_file_get_access never grabs a reference to it.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
