Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161515AbWJVAMB (ORCPT ); Sat, 21 Oct 2006 20:12:01 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1161514AbWJVAMA (ORCPT ); Sat, 21 Oct 2006 20:12:00 -0400 Received: from smtp.osdl.org ([65.172.181.4]:42379 "EHLO smtp.osdl.org") by vger.kernel.org with ESMTP id S1751756AbWJVAL7 (ORCPT ); Sat, 21 Oct 2006 20:11:59 -0400 Date: Sat, 21 Oct 2006 17:11:40 -0700 (PDT) From: Linus Torvalds To: James Morris cc: Josef Jeff Sipek , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, akpm@osdl.org, viro@ftp.linux.org.uk, hch@infradead.org Subject: Re: [PATCH 08 of 23] isofs: change uses of f_{dentry, vfsmnt} to use f_path In-Reply-To: Message-ID: References: <15a2d7465501c952a2af.1161411453@thor.fsl.cs.sunysb.edu> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1870 Lines: 51 On Sat, 21 Oct 2006, James Morris wrote: > > What about something like: > > static inline struct inode *fpath_ino(struct file *file) > { > return file->f_path.dentry->d_inode; > } Generally, unless it saves a _lot_ of typing, we've tried to avoid gratuitous hiding of details. And "ino" isn't a good name, it's something we've traditionally used for the inode _number_. So it would be "fpath_inode()" or "file_inode()" or something. As it is, the difference between file->f_dentry->d_inode fpath_inode(file) is not really enough of a win to merit hiding that it's doing two pointer dereferences. Now, whether the extra five characters ("path.") merit it, I don't know. I suspect not. If the line turns long, it's often more readable to just add a local variable or two, and do struct dentry *dentry = file->f_[path.]dentry; struct inode *inode = dentry->d_inode; which in some situations allow for other readability improvements too (eg maybe "dentry" or "inode" is used multiple times). If this was something where we'd expect things to change in the future, maybe it would be worth it for _that_ reason. That doesn't sound very likely, though - these things have been fairly stable, and even this patch is really about syntactic cleanup than any real change. Adding these kinds of "abstraction layers" is something that people are taught is good, but I personally tend to think that it makes it less obvious at the code level what the "costs" are. Unless you know things intimately, you really have no way of judging whether "fpath_inode()" is something expensive or not. I dunno. Linus - 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/