From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael Thompson" Subject: Re: [PATCH 6/13: eCryptfs] Superblock operations Date: Thu, 4 May 2006 10:00:45 -0500 Message-ID: References: <20060504031755.GA28257@hellewell.homeip.net> <20060504033829.GE28613@hellewell.homeip.net> <84144f020605040737k316fd5abva4476da69a65c084@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7BIT Cc: "Phillip Hellewell" , "Andrew Morton" , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, viro@ftp.linux.org.uk, mike@halcrow.us, mhalcrow@us.ibm.com, mcthomps@us.ibm.com, toml@us.ibm.com, yoder1@us.ibm.com, "James Morris" , "Stephen C. Tweedie" , "Erez Zadok" , "David Howells" Return-path: Received: from wx-out-0102.google.com ([66.249.82.193]:62527 "EHLO wx-out-0102.google.com") by vger.kernel.org with ESMTP id S1751489AbWEDPAp convert rfc822-to-8bit (ORCPT ); Thu, 4 May 2006 11:00:45 -0400 Received: by wx-out-0102.google.com with SMTP id t16so339272wxc for ; Thu, 04 May 2006 08:00:45 -0700 (PDT) To: "Pekka Enberg" In-Reply-To: <84144f020605040737k316fd5abva4476da69a65c084@mail.gmail.com> Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On 5/4/06, Pekka Enberg wrote: > Hi Phillip, > > Some comments below. > > On 5/4/06, Phillip Hellewell wrote: > > +kmem_cache_t *ecryptfs_inode_info_cache; > > Please use struct kmem_cache instead of the typedef. Please explain why. Looking at the source shows that kmem_cache_t is more widely used, and therefore seems to be the prefered way. > > + ecryptfs_printk(KERN_DEBUG, "Exit\n"); > > +} > > + > > +/** > > + * Set up the ecryptfs inode. > > + */ > > +static void ecryptfs_read_inode(struct inode *inode) > > +{ > > + ecryptfs_printk(KERN_DEBUG, "Enter; inode = [%p]\n", inode); > > + /* This is where we setup the self-reference in the vfs_inode's > > + * u.generic_ip. That way we don't have to walk the list again. */ > > + ECRYPTFS_INODE_TO_PRIVATE_SM(inode) = > > + list_entry(inode, struct ecryptfs_inode_info, vfs_inode); > > + ECRYPTFS_INODE_TO_LOWER(inode) = NULL; > > Hmm, ugly, please make the setters explicit instead. Curious, what exactly do you mean by this? I'm not sure what you mean by "setters". -- Michael C. Thompson Software-Engineer, IBM LTC Security