xfs
[Top] [All Lists]

Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values

To: Tom Haynes <thomas.haynes@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values
From: Christoph Hellwig <hch@xxxxxx>
Date: Sun, 11 Jan 2015 12:42:42 +0100
Cc: Christoph Hellwig <hch@xxxxxx>, "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Jeff Layton <jlayton@xxxxxxxxxxxxxxx>, linux-nfs@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150109230202.GB107259@kitty>
References: <1420561721-9150-1-git-send-email-hch@xxxxxx> <1420561721-9150-4-git-send-email-hch@xxxxxx> <20150109230202.GB107259@kitty>
User-agent: Mutt/1.5.17 (2007-11-01)
On Fri, Jan 09, 2015 at 03:02:02PM -0800, Tom Haynes wrote:
> >     DECODE_HEAD;
> > @@ -358,15 +373,10 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, 
> > u32 *bmval,
> >             dummy32 = be32_to_cpup(p++);
> >             switch (dummy32) {
> >             case NFS4_SET_TO_CLIENT_TIME:
> > -                   /* We require the high 32 bits of 'seconds' to be 0, 
> > and we ignore
> > -                      all 32 bits of 'nseconds'. */
> 
> Have you done away with these requirements?

No, the comment just go lost, I'll add it bacl.

> 
> > -                   READ_BUF(12);
> >                     len += 12;
> 
> I think this code makes it clear that the magic number 12 is the
> same on both lines. With the change, that gets lost.
> 
> Do I think that the 12 will ever change? No.
> 
> Do I think this becomes more "magic"? Yes.

Sure. but the whole counting the number to be decoded in setattr
is magic to start with.  I guess we could replace it with some magic
pointer arithmetic on argp->p, but is that really worth it?  Should
be a separate patch for sure.

<Prev in Thread] Current Thread [Next in Thread>