On Wed, 6 Jan 1999 11:34:48 +0000, Jamie Lokier
<lkd@tantalophile.demon.co.uk> said:
> There is a problem with the rename() system call in 2.1.131 on ext2fs.
> 1. The problem is that if source and destination are the same file, it
> returns zero ("success") but does not remove the source file. It
> should remove the source file.
Really?
>From my copy of POSIX.1:
"If the <old> argument and the <new> argument both refer to links to the
same existing file, the rename() function shall return successfully and
perform no other action."
>From singleunix:
"If the <old> argument and the <new> argument both refer to, and both
link to the same existing file, rename() returns successfully and
performs no other action."
Looks fine to me. Why do you think the behaviour should be different?
> While we're here, I noticed another bug:
> 2. If `dir_bh' is non-null, it is marked dirty but IS_SYNC is not
> honoured.
Correct. In fact, if you look through fs/ext2/namei.c, almost all of
the mark_inode_dirty() operations do not honour IS_SYNC. This is a
problem... Linus, is a fix for this likely to be a 2.2 candidate?
> and some other minor things:
> 3. `new_dir' is always marked dirty, version is incremented etc.
> These are not necessary when renaming over an existing file in
> a different directory. This is probably quite rare though.
We have to update the mtime in the new directory, which requires it to
be marked dirty.
> 4. There is no need to check `new_dir->i_nlink >= EXT2_LINK_MAX' when
> renaming within a directory.
> 5. If IS_SYNC, `old_bh' is written before `new_bh'. If there's
> a system failure in the middle, we'd much rather have two possible
> references to the filename rather than zero.
Bad, and...
> 6. If we're renaming within a directory and that directory IS_SYNC,
> it is written twice to disk and the rename takes twice as long.
> Once is enough.
...unpleasant. Both fixed in the patchlet below.
> 7. `old_dir->i_ctime' need not be updated if an inode is renamed
> within a directory, and the destination did not exist already.
> However, `old_dir->i_mtime' should always be updated. This is not
> easy to fix, because the call to `ext2_add_entry' updates
> `old_dir->i_ctime' itself.
No, ctime always needs to be updated any time anything in the inode is
modified. (Remember, it means change-time, not create-time.)
--Stephen
----------------------------------------------------------------
--- fs/ext2/namei.c.~1~ Fri Dec 18 15:09:35 1998
+++ fs/ext2/namei.c Fri Jan 8 23:50:40 1999
@@ -956,7 +956,6 @@
PARENT_INO(dir_bh->b_data) = le32_to_cpu(new_dir->i_ino);
mark_buffer_dirty(dir_bh, 1);
old_dir->i_nlink--;
- mark_inode_dirty(old_dir);
if (new_inode) {
new_inode->i_nlink--;
mark_inode_dirty(new_inode);
@@ -966,15 +965,17 @@
mark_inode_dirty(new_dir);
}
}
- mark_buffer_dirty(old_bh, 1);
- if (IS_SYNC(old_dir)) {
- ll_rw_block (WRITE, 1, &old_bh);
- wait_on_buffer (old_bh);
- }
mark_buffer_dirty(new_bh, 1);
if (IS_SYNC(new_dir)) {
ll_rw_block (WRITE, 1, &new_bh);
wait_on_buffer (new_bh);
+ }
+ if (old_bh != new_bh) {
+ mark_buffer_dirty(old_bh, 1);
+ if (IS_SYNC(old_dir)) {
+ ll_rw_block (WRITE, 1, &old_bh);
+ wait_on_buffer (old_bh);
+ }
}
/* Update the dcache */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/