On 11/10/14 11:41 AM, Carlos Maiolino wrote:
> Adds a new function named xfs_cross_rename(), responsible to handle requests
> from sys_renameat2() using RENAME_EXCHANGE flag.
>
> Changelog:
>
> V2: refactor xfs_cross_rename() to not duplicate code from xfs_rename()
Which tree is this against?
> Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.c | 315 +++++++++++++++++++++++++++++++++++------------------
> fs/xfs/xfs_inode.h | 8 +-
> fs/xfs/xfs_iops.c | 4 +-
> 3 files changed, 220 insertions(+), 107 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index fea3c92..bf09bfc 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2674,12 +2674,14 @@ xfs_rename(
> xfs_inode_t *src_ip,
> xfs_inode_t *target_dp,
> struct xfs_name *target_name,
> - xfs_inode_t *target_ip)
> + xfs_inode_t *target_ip,
> + unsigned int flags)
> {
> xfs_trans_t *tp = NULL;
> xfs_mount_t *mp = src_dp->i_mount;
> int new_parent; /* moving to a new dir */
> int src_is_directory; /* src_name is a directory */
> + int tgt_is_directory;
> int error;
> xfs_bmap_free_t free_list;
> xfs_fsblock_t first_block;
> @@ -2752,141 +2754,158 @@ xfs_rename(
> }
>
> /*
> - * Set up the target.
> - */
> - if (target_ip == NULL) {
> + * Handle RENAME_EXCHANGE flags
> + */
> + if (flags & RENAME_EXCHANGE) {
> /*
> - * If there's no space reservation, check the entry will
> - * fit before actually inserting it.
> + * target_ip will always exist if RENAME_EXCHANGE flag is set
> */
> - error = xfs_dir_canenter(tp, target_dp, target_name, spaceres);
> + tgt_is_directory = S_ISDIR(target_ip->i_d.di_mode);
> +
> + error = xfs_cross_rename(src_dp, src_name, src_ip, target_dp, target_name, target_ip,
> + new_parent, src_is_directory, tgt_is_directory,
> + &free_list, &first_block, tp, spaceres);
Ok, just some style drive-by comments, first -
EEK ;) 13 args and ~100 chars wide ;)
You could at least move some of those back to local vars in the
cross_rename function so you don't have to pass them on the stack.
It's also customary to pass the transaction pointer as the first arg,
i.e.:
+ error = xfs_cross_rename(tp, src_dp, src_name, src_ip,
+ target_dp, target_name, target_ip,
+ &free_list, &first_block, spaceres);
and:
xfs_cross_rename()
{
...
new_parent = (src_dp != target_dp);
src_is_directory = S_ISDIR(src_ip->i_d.di_mode);
tgt_is_directory = S_ISDIR(target_ip->i_d.di_mode);
...
}
There are a few other places where you go well beyond 80 chars, would rather
not do that if at all possible.
-Eric
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
