linux-kernel - Re: [PATCH 13/13] sysfs: Factor out sysfs_rename from sysfs_rename_dir and sysfs_move

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091104221957.GB21033@us.ibm.com>
Date:	Wed, 4 Nov 2009 16:19:57 -0600
From:	"Serge E. Hallyn" <serue@...ibm.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>,
	Kay Sievers <kay.sievers@...y.org>, Greg KH <greg@...ah.com>,
	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	linux-fsdevel@...r.kernel.org,
	Eric Dumazet <eric.dumazet@...il.com>,
	Benjamin LaHaise <bcrl@...et.ca>,
	"Eric W. Biederman" <ebiederm@...stanetworks.com>
Subject: Re: [PATCH 13/13] sysfs: Factor out sysfs_rename from
	sysfs_rename_dir and sysfs_move_dir

Quoting Eric W. Biederman (ebiederm@...ssion.com):
> From: Eric W. Biederman <ebiederm@...ssion.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@...nel.org>
> Signed-off-by: Eric W. Biederman <ebiederm@...stanetworks.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@...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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/