Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756582Ab3HFSEY (ORCPT ); Tue, 6 Aug 2013 14:04:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3034 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756078Ab3HFSEW (ORCPT ); Tue, 6 Aug 2013 14:04:22 -0400 Message-ID: <52013A9D.4010905@RedHat.com> Date: Tue, 06 Aug 2013 14:04:13 -0400 From: Steve Dickson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: "Myklebust, Trond" CC: Julia Lawall , David Quigley , "kernel-janitors@vger.kernel.org" , "linux-nfs@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] fs/nfs/inode.c: adjust code alignment References: <1375714059-29567-1-git-send-email-Julia.Lawall@lip6.fr> <1375714059-29567-2-git-send-email-Julia.Lawall@lip6.fr> <1375714793.7337.12.camel@leira.trondhjem.org> In-Reply-To: <1375714793.7337.12.camel@leira.trondhjem.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1951 Lines: 54 Hello, On 05/08/13 10:59, Myklebust, Trond wrote: > On Mon, 2013-08-05 at 16:47 +0200, Julia Lawall wrote: >> From: Julia Lawall >> >> Signed-off-by: Julia Lawall >> >> --- >> >> This patch adjusts the code so that the alignment matches the current >> semantics. I have no idea if it is the intended semantics, though. Should >> the call to nfs_setsecurity also be under the else? >> > >> fs/nfs/inode.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >> index af6e806..d8ad685 100644 >> --- a/fs/nfs/inode.c >> +++ b/fs/nfs/inode.c >> @@ -463,7 +463,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh >> *fh, struct nfs_fattr *fattr, st >> unlock_new_inode(inode); >> } else >> nfs_refresh_inode(inode, fattr); >> - nfs_setsecurity(inode, fattr, label); >> + nfs_setsecurity(inode, fattr, label); This call to nfs_setsecurity() is not needed. The security only needs to be set when the i-node is created... steved. >> dprintk("NFS: nfs_fhget(%s/%Ld fh_crc=0x%08x ct=%d)\n", >> inode->i_sb->s_id, >> (long long)NFS_FILEID(inode), > > Hi Julia, > > Thanks for pointing this out! Given that the 'then' clause of the if > statement already calls nfs_setsecurity before unlocking the inode, I > suspect that the above _should_ really be part of the 'else' clause. > > That said, I can't see that calling nfs_setsecurity twice on the inode > can cause any unintended side-effects, so I suggest that we rather queue > the patch up for inclusion in 3.12. > Steve and Dave, any comments? > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/