> On Aug 19, 2024, at 4:04 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Mon, 2024-08-19 at 19:50 +0000, Chuck Lever III wrote:
>>
>>
>>> On Aug 19, 2024, at 9:26 AM, Jeff Layton <jlayton@xxxxxxxxxx>
>>> 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.
cat'ing the two files together is the spec-recommended approach,
but it assumes you're generating the whole protocol at once.
Here it was just a quick and dirty way for me to build a
reproducer.
For an initial fs/nfsd/nfs4_1.x file, I recommend starting with
delstid.x, and then add the pieces of the NFSv4_1 XDR until
xdrgen and gcc can make proper sense of it.
I can take a stab at that if you like, and send you something
tomorrow?
Sidebar: We could go with all typedefs for structs, unions, and
enums. That would make C code generation easier. Something like:
typedef struct {
int64_t seconds;
uint32_t nseconds;
} nfstime4;
But like I said, I expect that approach might be frowned upon.
>>> 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.
I don't think cat'ing the .x files is /required/, but it was a
quick way to get started.
Having a working nfs4_1.x that can generate the small piece of
XDR code that we need, in a separate file that can be augmented
over time, I think, is a win. I don't see that anything so far
is preventing that.
--
Chuck Lever
