Re: [PATCH 1/3] nfsd: bring in support for delstid draft XDR encoding

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-08-19 at 19:50 +0000, Chuck Lever III wrote:
> 
> 
> > On Aug 19, 2024, at 9:26 AM, Jeff Layton <jlayton@kernel.org>
> > wrote:
> > 
> > I'm playing with the new version now and it seems to be much
> > improved.
> > Only two real bugs I've hit at this point:
> > 
> > 1/ Some of the struct specifications need to be typedefs as well.
> > For
> > instance, the delstid draft refers to "nfstime4", but the
> > autogenerated
> > struct definition doesn't have the typedef for it. It may be best
> > to
> > just add typedefs for all of these sorts of structs.
> 
> What's the specific symptom? I've been able to catenate nfs4_1.x
> and delstid.x, xdrgen builds the header and source without tossing
> any exceptions, and gcc compiles it without complaint.
> 


Basically, I was getting this when I'd convert nfs4_1.x to a header:

struct nfstime4 {
        int64_t seconds;
        uint32_t nseconds;
};

...but the delstid header has these:

typedef nfstime4 fattr4_time_deleg_access;

typedef nfstime4 fattr4_time_deleg_modify;


...nothing defined nfstime4 in this case.

> AFAICT, xdrgen will add "struct" where it's necessary.
> 
> I've been squirrelly about using "typedef" too often because
> the Linux kernel's coding style is to avoid C typedefs for
> shorthand structure names.
> 

Oh, ok. I didn't concatenate the files like you did and just generated
the delstid files separately from the nfs4_1 ones. I guess that throws
off the dependency tracking that you're doing here for typedefs.

> 
> > 2/ xdrgen_encode_nfstime4 want a pointer to the nfstime4, but the
> > autogenerated code for xdrgen_encode_fattr4_time_deleg_access and
> > xdrgen_encode_fattr4_time_deleg_modify try to pass it by value
> > instead.
> 
> Here's my generated copy of xdrgen_encode_fattr_time_deleg_access:
> 
> /* typedef fattr4_time_deleg_access */
> static bool
> __maybe_unused                                                     
> xdrgen_encode_fattr4_time_deleg_access(struct xdr_stream *xdr, const
> fattr4_time_deleg_access value)
> {
> 	/* (basic) */
> 	return xdrgen_encode_nfstime4(xdr, &value);
> };
> 
> Looks like it does the right thing...?

Probably another side-effect of it not knowing what to do with nfstime4
when I convert the delstid draft. Concatenating them seems unwieldy but
I guess that would work. I do like being able to keep generated code
from different files separate though.

-- 
Jeff Layton <jlayton@kernel.org>