lkml.org 
[lkml]   [2006]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08 of 23] isofs: change uses of f_{dentry, vfsmnt} to use f_path


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/

\
 
 \ /
  Last update: 2009-11-18 23:46    [W:0.039 / U:0.564 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site