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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
