I've attached a minor patch for ext2/namei.c adding a call to
shrink_dcache_parent in ext2_rmdir, and cleaning up some code.
A summary of changes:
(1) Unneeded tests for NULL dir inodes have been removed, as the call
depends on holding the inode semaphore.
(2) Some error exits have been streamlined to move the cleanup code off
of the common execution path.
(3) tests for user/group permissions involving fsuser() now consistently
test fsuser() last. This change was recommended recently in discussions
of tracking whether superuser privileges have actually been used.
(4) In two places I've flagged tests on inode i_count that I think
should be made using the dentry d_count, but would like someone more
familiar with ext2 to confirm this before changing the code.
Regards,
Bill
--------------2C9DFA0CC695333167B833CF
Content-Type: text/plain; charset=us-ascii; name="ext2_62-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="ext2_62-patch"
--- fs/ext2/namei.c.old Sun Aug 10 00:49:35 1997
+++ fs/ext2/namei.c Wed Nov 5 11:18:19 1997
@@ -353,8 +353,6 @@
struct ext2_dir_entry * de;
int err = -EIO;
- if (!dir)
- return -ENOENT;
/*
* N.B. Several error exits in ext2_new_inode don't set err.
*/
@@ -391,15 +389,13 @@
struct ext2_dir_entry * de;
int err = -EIO;
- if (!dir)
- return -ENOENT;
-
+ err = -ENAMETOOLONG;
if (dentry->d_name.len > EXT2_NAME_LEN)
- return -ENAMETOOLONG;
+ goto out;
inode = ext2_new_inode (dir, mode, &err);
if (!inode)
- return err;
+ goto out;
inode->i_uid = current->fsuid;
inode->i_mode = mode;
@@ -423,12 +419,8 @@
inode->i_rdev = to_kdev_t(rdev);
mark_inode_dirty(inode);
bh = ext2_add_entry (dir, dentry->d_name.name, dentry->d_name.len, &de, &err);
- if (!bh) {
- inode->i_nlink--;
- mark_inode_dirty(inode);
- iput(inode);
- return err;
- }
+ if (!bh)
+ goto out_no_entry;
de->inode = cpu_to_le32(inode->i_ino);
dir->i_version = ++event;
mark_buffer_dirty(bh, 1);
@@ -436,9 +428,17 @@
ll_rw_block (WRITE, 1, &bh);
wait_on_buffer (bh);
}
- brelse(bh);
d_instantiate(dentry, inode);
- return 0;
+ brelse(bh);
+ err = 0;
+out:
+ return err;
+
+out_no_entry:
+ inode->i_nlink--;
+ mark_inode_dirty(inode);
+ iput(inode);
+ goto out;
}
int ext2_mkdir(struct inode * dir, struct dentry * dentry, int mode)
@@ -446,23 +446,26 @@
struct inode * inode;
struct buffer_head * bh, * dir_block;
struct ext2_dir_entry * de;
- int err = -EIO;
+ int err;
+ err = -ENAMETOOLONG;
if (dentry->d_name.len > EXT2_NAME_LEN)
- return -ENAMETOOLONG;
+ goto out;
+ err = -EMLINK;
if (dir->i_nlink >= EXT2_LINK_MAX)
- return -EMLINK;
+ goto out;
+ err = -EIO;
inode = ext2_new_inode (dir, S_IFDIR, &err);
if (!inode)
- return err;
+ goto out;
inode->i_op = &ext2_dir_inode_operations;
inode->i_size = inode->i_sb->s_blocksize;
dir_block = ext2_bread (inode, 0, 1, &err);
if (!dir_block) {
- inode->i_nlink--;
+ inode->i_nlink--; /* is this nlink == 0? */
mark_inode_dirty(inode);
iput (inode);
return err;
@@ -486,12 +489,8 @@
inode->i_mode |= S_ISGID;
mark_inode_dirty(inode);
bh = ext2_add_entry (dir, dentry->d_name.name, dentry->d_name.len, &de, &err);
- if (!bh) {
- inode->i_nlink = 0;
- mark_inode_dirty(inode);
- iput (inode);
- return err;
- }
+ if (!bh)
+ goto out_no_entry;
de->inode = cpu_to_le32(inode->i_ino);
dir->i_version = ++event;
mark_buffer_dirty(bh, 1);
@@ -503,7 +502,15 @@
mark_inode_dirty(dir);
d_instantiate(dentry, inode);
brelse (bh);
- return 0;
+ err = 0;
+out:
+ return err;
+
+out_no_entry:
+ inode->i_nlink = 0;
+ mark_inode_dirty(inode);
+ iput (inode);
+ goto out;
}
/*
@@ -572,25 +579,23 @@
struct buffer_head * bh;
struct ext2_dir_entry * de;
- if (!dir)
- return -ENOENT;
- inode = NULL;
+ retval = -ENAMETOOLONG;
if (dentry->d_name.len > EXT2_NAME_LEN)
- return -ENAMETOOLONG;
+ goto out;
- bh = ext2_find_entry (dir, dentry->d_name.name, dentry->d_name.len, &de);
retval = -ENOENT;
+ bh = ext2_find_entry (dir, dentry->d_name.name, dentry->d_name.len, &de);
if (!bh)
goto end_rmdir;
- retval = -EPERM;
- inode = dentry->d_inode;
+ inode = dentry->d_inode;
if (inode->i_sb->dq_op)
inode->i_sb->dq_op->initialize (inode, -1);
- if ((dir->i_mode & S_ISVTX) && !fsuser() &&
- current->fsuid != inode->i_uid &&
- current->fsuid != dir->i_uid)
+ retval = -EPERM;
+ if ((dir->i_mode & S_ISVTX) &&
+ current->fsuid != inode->i_uid &&
+ current->fsuid != dir->i_uid && !fsuser())
goto end_rmdir;
if (inode == dir) /* we may not delete ".", but "../dir" is ok */
goto end_rmdir;
@@ -606,11 +611,19 @@
goto end_rmdir;
down(&inode->i_sem);
+ /*
+ * Prune any child dentries so that this dentry becomes negative.
+ */
+ if (dentry->d_count > 1) {
+ printk("ext2_rmdir: d_count=%d, pruning\n", dentry->d_count);
+ shrink_dcache_parent(dentry);
+ }
if (!empty_dir (inode))
retval = -ENOTEMPTY;
else if (le32_to_cpu(de->inode) != inode->i_ino)
retval = -ENOENT;
else {
+ /* N.B. Shouldn't this test the dentry count? */
if (inode->i_count > 1) {
/*
* Are we deleting the last instance of a busy directory?
@@ -646,6 +659,7 @@
end_rmdir:
brelse (bh);
+out:
return retval;
}
@@ -656,11 +670,11 @@
struct buffer_head * bh;
struct ext2_dir_entry * de;
- retval = -ENOENT;
- inode = NULL;
+ retval = -ENAMETOOLONG;
if (dentry->d_name.len > EXT2_NAME_LEN)
- return -ENAMETOOLONG;
+ goto out;
+ retval = -ENOENT;
bh = ext2_find_entry (dir, dentry->d_name.name, dentry->d_name.len, &de);
if (!bh)
goto end_unlink;
@@ -674,9 +688,9 @@
goto end_unlink;
if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
goto end_unlink;
- if ((dir->i_mode & S_ISVTX) && !fsuser() &&
+ if ((dir->i_mode & S_ISVTX) &&
current->fsuid != inode->i_uid &&
- current->fsuid != dir->i_uid)
+ current->fsuid != dir->i_uid && !fsuser())
goto end_unlink;
retval = -EIO;
@@ -708,13 +722,14 @@
end_unlink:
brelse (bh);
+out:
return retval;
}
int ext2_symlink (struct inode * dir, struct dentry *dentry, const char * symname)
{
struct ext2_dir_entry * de;
- struct inode * inode = NULL;
+ struct inode * inode;
struct buffer_head * bh = NULL, * name_block = NULL;
char * link;
int i, l, err = -EIO;
@@ -758,12 +773,8 @@
mark_inode_dirty(inode);
bh = ext2_add_entry (dir, dentry->d_name.name, dentry->d_name.len, &de, &err);
- if (!bh) {
- inode->i_nlink--;
- mark_inode_dirty(inode);
- iput (inode);
- return err;
- }
+ if (!bh)
+ goto out_no_entry;
de->inode = cpu_to_le32(inode->i_ino);
dir->i_version = ++event;
mark_buffer_dirty(bh, 1);
@@ -773,7 +784,15 @@
}
brelse (bh);
d_instantiate(dentry, inode);
- return 0;
+ err = 0;
+out:
+ return err;
+
+out_no_entry:
+ inode->i_nlink--;
+ mark_inode_dirty(inode);
+ iput (inode);
+ goto out;
}
int ext2_link (struct inode * inode, struct inode * dir, struct dentry *dentry)
@@ -888,10 +907,9 @@
new_inode->i_sb->dq_op->initialize (new_inode, -1);
}
}
- if (new_inode == old_inode) {
- retval = 0;
+ retval = 0;
+ if (new_inode == old_inode)
goto end_rename;
- }
if (new_inode && S_ISDIR(new_inode->i_mode)) {
retval = -EISDIR;
if (!S_ISDIR(old_inode->i_mode))
@@ -903,6 +921,7 @@
if (!empty_dir (new_inode))
goto end_rename;
retval = -EBUSY;
+ /* N.B. shouldn't this test the dentry count? */
if (new_inode->i_count > 1)
goto end_rename;
}
--------------2C9DFA0CC695333167B833CF--