> + switch (seek->seek_whence) { > + case NFS4_CONTENT_DATA: > + seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_DATA); > + break; > + case NFS4_CONTENT_HOLE: > + seek->seek_pos = vfs_llseek(file, seek->seek_offset, SEEK_HOLE); > + break; > + default: > + status = nfserr_union_notsupp; > + goto out; > + } nipick: maybe just assign pos in the switch, and have a single vfs_llseek call? Also this might want a comment that vfs_llseek is changing file->f_pos, but nothing in NFSD should ever rely on file->f_pos. > static __be32 > +nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr, > + struct nfsd4_seek *seek) > +{ > + __be32 *p; > + > + if (nfserr) > + return nfserr; > + > + p = xdr_reserve_space(&resp->xdr, 12); nipick: can you replace the 12 by a "4 + 8"? I think having one literal for each field later encoded helps reading the XDR code a lot. And although it might not matter for a trivial encoder like this it set standards for future copy and paste code. -- 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