Re: [PATCH 3/3] NFSD: Tighten header includes in localio.c

NeilBrown <[email protected]> Tue, 28 Jul 2026 10:33:34 +1000
Newsgroups gmane.linux.nfs
Message-ID <[email protected]>
On Mon, 27 Jul 2026, Chuck Lever wrote:
> As a prerequisite to converting NFSD to use xdrgen more broadly,
> NFSD source files should not depend on NFS client headers.
> fs/nfsd/localio.c is server-side LOCALIO code, yet it pulled in
> three of them: <linux/nfs_fs.h>, the client inode header (struct
> nfs_inode, NFS_I(), writeback helpers), which server code never
> uses; <linux/nfs_xdr.h>, whose only referenced symbol is
> decode_opaque_fixed(), a static inline that exists to remap the
> error return to -EIO for client call sites; and the catch-all
> <linux/nfs.h>.
>=20
> Convert the UUID decoder to call the canonical SUNRPC primitive
> xdr_stream_decode_opaque_fixed() directly. It is shared by client
> and server, performs the identical bounds check, and is already
> reachable through <linux/sunrpc/clnt.h>. With the wrapper gone,
> localio.c references no symbol from <linux/nfs_xdr.h>, and with
> that header gone, none of the NFSv3 definitions its structs embed
> are needed here.
>=20
> Drop all three client includes and add what the file actually
> uses: struct nfs_fh comes from <linux/nfs_fh.h>, included directly
> rather than through nfslocalio.h's conditional re-export, and
> NFS4_FHSIZE from <linux/nfs4.h>. enum nfs_stat and
> nfs_stat_to_errno continue to come from the already-included
> <linux/nfs_common.h>.

I think it would be better to not include nfs4.h because it doesn't seem
particularly relevant to localio.
Instead, replace NFS4_FHSIZE in localio.c with NFS_MAXFHSIZE which is
defined in nfs_fh.h.  I think that macro is a better fit for what is
actually wanted.

Otherwise this all looks good and sensible.

Thanks,
NeilBrown

>=20
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  fs/nfsd/localio.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>=20
> diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
> index c458c01e9478..4110be02b750 100644
> --- a/fs/nfsd/localio.c
> +++ b/fs/nfsd/localio.c
> @@ -11,11 +11,10 @@
>  #include <linux/exportfs.h>
>  #include <linux/sunrpc/svcauth.h>
>  #include <linux/sunrpc/clnt.h>
> -#include <linux/nfs.h>
> +#include <linux/nfs4.h>
>  #include <linux/nfs_common.h>
> +#include <linux/nfs_fh.h>
>  #include <linux/nfslocalio.h>
> -#include <linux/nfs_fs.h>
> -#include <linux/nfs_xdr.h>
>  #include <linux/string.h>
> =20
>  #include "nfsd.h"
> @@ -179,7 +178,7 @@ static bool localio_decode_uuidarg(struct svc_rqst *rqs=
tp,
>  	struct localio_uuidarg *argp =3D rqstp->rq_argp;
>  	u8 uuid[UUID_SIZE];
> =20
> -	if (decode_opaque_fixed(xdr, uuid, UUID_SIZE))
> +	if (xdr_stream_decode_opaque_fixed(xdr, uuid, UUID_SIZE) < 0)
>  		return false;
>  	import_uuid(&argp->uuid, uuid);
> =20
> --=20
> 2.54.0
>=20
>=20