lkml.org 
[lkml]   [2008]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [BUGFIX][PATCH 3/3] configfs: Fix failing symlink() making rmdir() fail
On Tue, Jun 17, 2008 at 07:37:23PM +0200, Louis Rilling wrote:

Ok, I can see some of why I hated this.

> +
> spin_lock(&configfs_dirent_lock);
> - if (target_sd->s_type & CONFIGFS_USET_DROPPING) {
> - spin_unlock(&configfs_dirent_lock);
> - config_item_put(item);
> - kfree(sl);
> - return -EPERM;
> - }
> - list_add(&sl->sl_list, &target_sd->s_links);
> + /*
> + * Force rmdir() of parent_item to wait until we know if we
> + * succeed.
> + */
> + parent_sd->s_type |= CONFIGFS_USET_ATTACHING;
> spin_unlock(&configfs_dirent_lock);
> +
> ret = configfs_create_link(sl, parent_item->ci_dentry,
> dentry);
> - if (ret) {
> - spin_lock(&configfs_dirent_lock);
> - list_del_init(&sl->sl_list);
> +
> + spin_lock(&configfs_dirent_lock);
> + parent_sd->s_type &= ~CONFIGFS_USET_ATTACHING;
> +
> + if (ret || target_sd->s_type & CONFIGFS_USET_DROPPING) {

Use parenthesis. Actually, separate out the error cases.

> + struct configfs_dirent *sd = NULL;
> +
> + if (!ret) {
> + sd = dentry->d_fsdata;
> + list_del_init(&sd->s_sibling);
> + }
> spin_unlock(&configfs_dirent_lock);
> +
> + if (!ret) {
> + configfs_drop_dentry(sd, dentry->d_parent);
> + d_delete(dentry);
> + configfs_put(sd);
> + }

This open-code of the VFS munging is ripe to break when the VFS
changes or other configfs changes happen. The real issue is that you
are reimplementing the core of configfs_unlink(). Note how the core VFS
munging of configfs_rmdir() is separated out so that configfs_mkdir()
can also use it in the failure case? Do the same with unlink() and it
will read much better ("if (DROPPING) configfs_delete_link()"). Call it
configfs_remove_link() or configfs_delete_link().
Joel

--

Life's Little Instruction Book #337

"Reread your favorite book."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127


\
 
 \ /
  Last update: 2008-06-20 00:25    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site