xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values
From: Tom Haynes <thomas.haynes@xxxxxxxxxxxxxxx>
Date: Sun, 11 Jan 2015 18:53:44 -0500
Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Jeff Layton <jlayton@xxxxxxxxxxxxxxx>, linux-nfs@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150111114242.GA11939@xxxxxx>
References: <1420561721-9150-1-git-send-email-hch@xxxxxx> <1420561721-9150-4-git-send-email-hch@xxxxxx> <20150109230202.GB107259@kitty> <20150111114242.GA11939@xxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Sun, Jan 11, 2015 at 12:42:42PM +0100, Christoph Hellwig wrote:
> On Fri, Jan 09, 2015 at 03:02:02PM -0800, Tom Haynes wrote:
> > 
> > > -                 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.  

Agreed.

> I guess we could replace it with some magic
> pointer arithmetic on argp->p, but is that really worth it?  

Which is why I asked the leading questions. I see both sides,
but ultimately it is a nit considering the rest of the abuse.

I'm fine with you deciding it is still magic overall.

> Should
> be a separate patch for sure.

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