[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130115194330.GF17719@thunk.org>
Date: Tue, 15 Jan 2013 14:43:30 -0500
From: Theodore Ts'o <tytso@....edu>
To: Darren Hart <dvhart@...radead.org>
Cc: linux-ext4@...r.kernel.org, adilger@...ger.ca, sgw@...ux.intel.com,
darrick.wong@...cle.com
Subject: Re: [PATCH 1/3] lib/ext2fs: Add ext2fs_symlink
I'll fix up the this patch before I commit it, but this is a perfect
exhibit about why I request that code submissions come with test
cases. It turns out that there were a couple of problems with
ext2fs_symlink(), that showed up very quickly as soon as I started
writing a test case (where it's important to run e2fsck on the
resulting file system after creating the symlinks --- e2fsck is a
wonderful rep invariant checkers for ext[234] file systems. :-)
*) i_blocks must be set to 0 for fast symlinks
*) The last argument of ext2fs_inode_alloc_stats() indicates whether
the new inode is a directory or not. So when you cut and pasted
the code from ext2fs_mkdir(), it needed to be changed.
*) Zeroing the entire block before setting the symlink in the case
where it needs to use an external data block makes it a lot
easier to write the regression test.
So here's the patch I needed to apply on top of your submission....
- Ted
diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c
index fb8b91b..da6e3a8 100644
--- a/lib/ext2fs/symlink.c
+++ b/lib/ext2fs/symlink.c
@@ -78,7 +78,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
memset(&inode, 0, sizeof(struct ext2_inode));
inode.i_mode = LINUX_S_IFLNK | 0777;
inode.i_uid = inode.i_gid = 0;
- ext2fs_iblk_set(fs, &inode, 1);
+ ext2fs_iblk_set(fs, &inode, fastlink ? 0 : 1);
inode.i_links_count = 1;
inode.i_size = target_len;
/* The time fields are set by ext2fs_write_new_inode() */
@@ -88,6 +88,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
strcpy((char *)&inode.i_block, target);
} else {
/* Slow symlinks, target stored in the first block */
+ memset(block_buf, 0, fs->blocksize);
strcpy(block_buf, target);
if (fs->super->s_feature_incompat &
EXT3_FEATURE_INCOMPAT_EXTENTS) {
@@ -149,7 +150,7 @@ errcode_t ext2fs_symlink(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t ino,
*/
if (!fastlink)
ext2fs_block_alloc_stats2(fs, blk, +1);
- ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
+ ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
cleanup:
if (block_buf)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists