Re: [PATCH 13/13] sysfs: Factor out sysfs_rename from sysfs_rename_dir and sysfs_move_dir

Serge E. Hallyn
2009-11-04T22:19:56Z

Quoting Eric W. Biederman (ebiederm@xmission.com):
> From: Eric W. Biederman <ebiederm@xmission.com>
>
> These two functions do 90% of the same work and it doesn't significantly
> obfuscate the function to allow both the parent dir and the name to change
> at the same time. So merge them together to simplify maintenance, and
> increase testing.
>
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>

Based on just this patchset it seems sysfs_rename() could be static,
but since it isn't static I assume your later patchset will use it
outside fs/sysfs/dir.c?

Acked-by: Serge Hallyn <serue@us.ibm.com>

> ---
> fs/sysfs/dir.c | 62 +++++++++++++++++++++++++----------------------------
> fs/sysfs/sysfs.h | 3 ++
> 2 files changed, 32 insertions(+), 33 deletions(-)
>
> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
> index 0b60212..e1a86d1 100644
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -760,30 +760,42 @@ void sysfs_remove_dir(struct kobject * kobj)
> __sysfs_remove_dir(sd);
> }
>
> -int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
> +int sysfs_rename(struct sysfs_dirent *sd,
> + struct sysfs_dirent *new_parent_sd, const char *new_name)
> {
> - struct sysfs_dirent *sd = kobj->sd;
> const char *dup_name = NULL;
> int error;
>
> mutex_lock(&sysfs_mutex);
>
> error = 0;
> - if (strcmp(sd->s_name, new_name) == 0)
> + if ((sd->s_parent == new_parent_sd) &&
> + (strcmp(sd->s_name, new_name) == 0))
> goto out; /* nothing to rename */
>
> error = -EEXIST;
> - if (sysfs_find_dirent(sd->s_parent, new_name))
> + if (sysfs_find_dirent(new_parent_sd, new_name))
> goto out;
>
> /* rename sysfs_dirent */
> - error = -ENOMEM;
> - new_name = dup_name = kstrdup(new_name, GFP_KERNEL);
> - if (!new_name)
> - goto out;
> + if (strcmp(sd->s_name, new_name) != 0) {
> + error = -ENOMEM;
> + new_name = dup_name = kstrdup(new_name, GFP_KERNEL);
> + if (!new_name)
> + goto out;
> +
> + dup_name = sd->s_name;
> + sd->s_name = new_name;
> + }
>
> - dup_name = sd->s_name;
> - sd->s_name = new_name;
> + /* Remove from old parent's list and insert into new parent's list. */
> + if (sd->s_parent != new_parent_sd) {
> + sysfs_unlink_sibling(sd);
> + sysfs_get(new_parent_sd);
> + sysfs_put(sd->s_parent);
> + sd->s_parent = new_parent_sd;
> + sysfs_link_sibling(sd);
> + }
>
> error = 0;
> out:
> @@ -792,37 +804,21 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
> return error;
> }
>
> +int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
> +{
> + return sysfs_rename(kobj->sd, kobj->sd->s_parent, new_name);
> +}
> +
> int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
> {
> struct sysfs_dirent *sd = kobj->sd;
> struct sysfs_dirent *new_parent_sd;
> - int error;
>
> BUG_ON(!sd->s_parent);
> -
> - mutex_lock(&sysfs_mutex);
> - new_parent_sd = (new_parent_kobj && new_parent_kobj->sd) ?
> + new_parent_sd = new_parent_kobj && new_parent_kobj->sd ?
> new_parent_kobj->sd : &sysfs_root;
>
> - error = 0;
> - if (sd->s_parent == new_parent_sd)
> - goto out; /* nothing to move */
> -
> - error = -EEXIST;
> - if (sysfs_find_dirent(new_parent_sd, sd->s_name))
> - goto out;
> -
> - /* Remove from old parent's list and insert into new parent's list. */
> - sysfs_unlink_sibling(sd);
> - sysfs_get(new_parent_sd);
> - sysfs_put(sd->s_parent);
> - sd->s_parent = new_parent_sd;
> - sysfs_link_sibling(sd);
> -
> - error = 0;
> -out:
> - mutex_unlock(&sysfs_mutex);
> - return error;
> + return sysfs_rename(sd, new_parent_sd, sd->s_name);
> }
>
> /* Relationship between s_mode and the DT_xxx types */
> diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
> index 98a15bf..ca52e7b 100644
> --- a/fs/sysfs/sysfs.h
> +++ b/fs/sysfs/sysfs.h
> @@ -130,6 +130,9 @@ int sysfs_create_subdir(struct kobject *kobj, const char *name,
> struct sysfs_dirent **p_sd);
> void sysfs_remove_subdir(struct sysfs_dirent *sd);
>
> +int sysfs_rename(struct sysfs_dirent *sd,
> + struct sysfs_dirent *new_parent_sd, const char *new_name);
> +
> static inline struct sysfs_dirent *__sysfs_get(struct sysfs_dirent *sd)
> {
> if (sd) {
> --
> 1.6.5.2.143.g8cc62
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html