On Tue, Apr 08, 2008 at 02:50:17PM +1000, Barry Naujok wrote:
> +static inline struct xfs_name *
> +xfs_dentry_name(
> + struct xfs_name *namep,
> + struct dentry *dentry)
> +{
> + namep->name = dentry->d_name.name;
> + namep->len = dentry->d_name.len;
> + return namep;
> +}
xfs_dentry_to_name()
> @@ -246,20 +256,19 @@ xfs_cleanup_inode(
> struct dentry *dentry,
> int mode)
> {
> - struct dentry teardown = {};
> + struct xfs_name name;
I'd leave the variable name as 'teardown'. ie.
+ struct xfs_name teardown;
> @@ -371,12 +383,13 @@ xfs_vn_lookup(
> struct nameidata *nd)
> {
> struct xfs_inode *cip;
> + struct xfs_name name;
> int error;
>
> if (dentry->d_name.len >= MAXNAMELEN)
> return ERR_PTR(-ENAMETOOLONG);
>
> - error = xfs_lookup(XFS_I(dir), dentry, &cip);
> + error = xfs_lookup(XFS_I(dir), xfs_dentry_name(&name, dentry), &cip);
I'd prefer this to be separate operations:
+ xfs_dentry_to_name(&name, dentry);
+ error = xfs_lookup(XFS_I(dir), &name, &cip);
that way it's clear that we're passing name down the stack....
Same for all the others....
> @@ -489,9 +509,12 @@ xfs_vn_rename(
> struct dentry *ndentry)
> {
> struct inode *new_inode = ndentry->d_inode;
> + struct xfs_name oname, nname;
+ struct xfs_name oname;
+ struct xfs_name nname;
......
> - args.hashval = xfs_da_hashname(name, namelen);
> + args.name = name->name;
> + args.namelen = name->len;
> + args.hashval = xfs_da_hashname(name->name, name->len);
Seems like future work would be to drive the xfs_name_t further
into these leaf functions....
> @@ -83,17 +83,16 @@ int xfs_rename_skip, xfs_rename_nskip;
> */
> STATIC int
> xfs_lock_for_rename(
> - xfs_inode_t *dp1, /* old (source) directory inode */
> - xfs_inode_t *dp2, /* new (target) directory inode */
> - bhv_vname_t *vname1,/* old entry name */
> - bhv_vname_t *vname2,/* new entry name */
> - xfs_inode_t **ipp1, /* inode of old entry */
> - xfs_inode_t **ipp2, /* inode of new entry, if it
> + xfs_inode_t *dp1, /* in: old (source) directory inode */
> + xfs_inode_t *dp2, /* in: new (target) directory inode */
> + xfs_name_t *name2, /* in: new entry name */
> + xfs_inode_t **ipp1, /* in/out: inode of old entry */
> + xfs_inode_t **ipp2, /* out: inode of new entry, if it
> already exists, NULL otherwise. */
> - xfs_inode_t **i_tab,/* array of inode returned, sorted */
> - int *num_inodes) /* number of inodes in array */
> + xfs_inode_t **i_tab,/* out: array of inode returned, sorted */
> + int *num_inodes) /* out: number of inodes in array */
> {
> - xfs_inode_t *ip1 = VNAME_TO_INODE(vname1);
> + xfs_inode_t *ip1;
> xfs_inode_t *ip2, *temp;
> xfs_ino_t inum1, inum2;
> int error;
> @@ -101,6 +100,7 @@ xfs_lock_for_rename(
> uint lock_mode;
> int diff_dirs = (dp1 != dp2);
>
> + ip1 = *ipp1;
> ip2 = NULL;
ASSERT(ip1);
.....
> * Initially assume that the file does not exist and
> * reserve the resources for that case. If that is not
> @@ -1883,7 +1879,7 @@ xfs_create(
> if (error)
> goto error_return;
>
> - if (resblks == 0 && (error = xfs_dir_canenter(tp, dp, name,
> namelen)))
> + if (resblks == 0 && (error = xfs_dir_canenter(tp, dp, name)))
> goto error_return;
If you're touching lines like that, you should restructure it at the
same time to match accepted style more closely.
if (!resblks) {
error = xfs_dir_canenter(tp, dp, name);
if (error)
goto error_return;
}
> @@ -2567,12 +2558,12 @@ xfs_link(
> }
>
> if (resblks == 0 &&
> - (error = xfs_dir_canenter(tp, tdp, target_name, target_namelen)))
> + (error = xfs_dir_canenter(tp, tdp, target_name)))
> goto error_return;
Same again.
> @@ -2719,7 +2708,7 @@ xfs_mkdir(
> goto error_return;
>
> if (resblks == 0 &&
> - (error = xfs_dir_canenter(tp, dp, dir_name, dir_namelen)))
> + (error = xfs_dir_canenter(tp, dp, dir_name)))
> goto error_return;
more!
> @@ -3201,7 +3184,7 @@ xfs_symlink(
> * Check for ability to enter directory entry, if no space reserved.
> */
> if (resblks == 0 &&
> - (error = xfs_dir_canenter(tp, dp, link_name, link_namelen)))
> + (error = xfs_dir_canenter(tp, dp, link_name)))
> goto error_return;
how many times do we do almost the same thing? :P
Looks pretty good, otherwise....
Cheers,
Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
|