From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Haynes Subject: Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values Date: Sun, 11 Jan 2015 18:53:44 -0500 Message-ID: <20150111235344.GA11746@slacker.internal.excfb.com> References: <1420561721-9150-1-git-send-email-hch@lst.de> <1420561721-9150-4-git-send-email-hch@lst.de> <20150109230202.GB107259@kitty> <20150111114242.GA11939@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "J. Bruce Fields" , Jeff Layton , linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, xfs-VZNHf3L845pBDgjK7y7TUQ@public.gmane.org To: Christoph Hellwig Return-path: Content-Disposition: inline In-Reply-To: <20150111114242.GA11939-jcswGhMUV9g@public.gmane.org> Sender: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org 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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id BE7417F50 for ; Sun, 11 Jan 2015 17:53:52 -0600 (CST) Received: from cuda.sgi.com (cuda3.sgi.com [192.48.176.15]) by relay1.corp.sgi.com (Postfix) with ESMTP id AD53D8F8035 for ; Sun, 11 Jan 2015 15:53:49 -0800 (PST) Received: from mail-yh0-f45.google.com (mail-yh0-f45.google.com [209.85.213.45]) by cuda.sgi.com with ESMTP id EqDpHDS6lCanbAe1 (version=TLSv1 cipher=RC4-SHA bits=128 verify=NO) for ; Sun, 11 Jan 2015 15:53:48 -0800 (PST) Received: by mail-yh0-f45.google.com with SMTP id f10so8376915yha.4 for ; Sun, 11 Jan 2015 15:53:48 -0800 (PST) Date: Sun, 11 Jan 2015 18:53:44 -0500 From: Tom Haynes Subject: Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values Message-ID: <20150111235344.GA11746@slacker.internal.excfb.com> References: <1420561721-9150-1-git-send-email-hch@lst.de> <1420561721-9150-4-git-send-email-hch@lst.de> <20150109230202.GB107259@kitty> <20150111114242.GA11939@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150111114242.GA11939@lst.de> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Christoph Hellwig Cc: "J. Bruce Fields" , linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, Jeff Layton , xfs@oss.sgi.com 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. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: linux-nfs-owner@vger.kernel.org Received: from mail-yh0-f52.google.com ([209.85.213.52]:58951 "EHLO mail-yh0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750955AbbAKXxs (ORCPT ); Sun, 11 Jan 2015 18:53:48 -0500 Received: by mail-yh0-f52.google.com with SMTP id z6so8370406yhz.11 for ; Sun, 11 Jan 2015 15:53:48 -0800 (PST) Date: Sun, 11 Jan 2015 18:53:44 -0500 From: Tom Haynes To: Christoph Hellwig Cc: "J. Bruce Fields" , Jeff Layton , linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com Subject: Re: [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values Message-ID: <20150111235344.GA11746@slacker.internal.excfb.com> References: <1420561721-9150-1-git-send-email-hch@lst.de> <1420561721-9150-4-git-send-email-hch@lst.de> <20150109230202.GB107259@kitty> <20150111114242.GA11939@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20150111114242.GA11939@lst.de> Sender: linux-nfs-owner@vger.kernel.org List-ID: 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.