> On Nov 23, 2020, at 2:18 PM, bfields@xxxxxxxxxxxx wrote: > > On Fri, Nov 20, 2020 at 03:34:12PM -0500, Chuck Lever wrote: >> @@ -396,7 +281,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval, >> READ_BUF(4); len += 4; >> nace = be32_to_cpup(p++); >> >> - if (nace > compoundargs_bytes_left(argp)/20) >> + if (nace > xdr_stream_remaining(argp->xdr) / sizeof(struct nfs4_ace)) > > Picky C question: is the compiler guaranteed to pack that struct in the > obvious way? > > That aside, I'm not comfortable assuming the struct could never change > to, say, include something that's useful during processing but doesn't > appear on the wire. OK, but you also don't like naked integers without documentation. I can leave this as "xdr_stream_remaining() / 20" if you prefer. Or we could define a macro somewhere that makes this code a little more self-documenting. > Also, that change isn't actually logically related to the rest of the > patch, so it's the sort of thing I'd expect separated out. > > --b. -- Chuck Lever