Date: Mon, 19 Jan 2009 22:45:01 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Stanislav Sedov <stas@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r187395 - head/sys/gnu/fs/ext2fs Message-ID: <20090119220943.P37158@delplex.bde.org> In-Reply-To: <200901181404.n0IE4uXw075698@svn.freebsd.org> References: <200901181404.n0IE4uXw075698@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 18 Jan 2009, Stanislav Sedov wrote: > Author: stas > Date: Sun Jan 18 14:04:56 2009 > New Revision: 187395 > URL: http://svn.freebsd.org/changeset/base/187395 > > Log: > - Obtain inode sizes and location of the first inode based on the contents > of superblock rather than using hardcoded values. This fixes ext2fs on > filesystems with inode sized other than 128. > > Submitted by: Alex Lyashkov <Alexey.Lyashkov@Sun.COM> (based on) > MFC after: 2 weeks This was not suitable for committing verbatim. > Modified: head/sys/gnu/fs/ext2fs/ext2_fs.h > ============================================================================== > --- head/sys/gnu/fs/ext2fs/ext2_fs.h Sun Jan 18 13:04:38 2009 (r187394) > +++ head/sys/gnu/fs/ext2fs/ext2_fs.h Sun Jan 18 14:04:56 2009 (r187395) > @@ -150,8 +150,8 @@ > #else /* !notyet */ ^^^^^^^^^^^^^^^^^^^ > #define EXT2_INODES_PER_BLOCK(s) ((s)->s_inodes_per_block) > /* Should be sizeof(struct ext2_inode): */ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > -#define EXT2_INODE_SIZE 128 > -#define EXT2_FIRST_INO 11 > +#define EXT2_INODE_SIZE(s) ((s)->s_inode_size) > +#define EXT2_FIRST_INO(s) ((s)->s_first_inode) > #endif /* notyet */ Please read the code before changing parts of it. The comment is now just wrong. One value that EXT2_INODE_SIZE certainly should _not_ be is the fixed size of our data structure, since we are now trying to support a variable size and even have a parameter to do that. The ifdef is now more bogus than before. The "!notyet" part now essentially duplicates the old unchanged "notyet" part, except for having bugs not present in the old version. This ifdef is a FreeBSD hack which should have been removed. Linux ext2fs just has the "notyet" part. Here is complete old ifdef: % #ifdef notyet % #ifdef __KERNEL__ % #define EXT2_ADDR_PER_BLOCK_BITS(s) ((s)->u.ext2_sb.s_addr_per_block_bits) % #define EXT2_INODE_SIZE(s) ((s)->u.ext2_sb.s_inode_size) % #define EXT2_FIRST_INO(s) ((s)->u.ext2_sb.s_first_ino) This is literally the same as in Linux. It cannot be used directly, now for only minor reasons: - FreeBSD doesn't have "u.ext2.sb". After deleting "u.ext2_sb." from the above, it seems to be right. - this was already done in the !notyet part for the 1st macro - your change does this for the other 2 macros, but this is not enough -- see below. % #else % #define EXT2_INODE_SIZE(s) (((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \ % EXT2_GOOD_OLD_INODE_SIZE : \ % (s)->s_inode_size) % #define EXT2_FIRST_INO(s) (((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \ % EXT2_GOOD_OLD_FIRST_INO : \ % (s)->s_first_ino) s_inode_size and s_first_ino cannot be used directly because they aren't supported by EXT2_GOOD_OLD_REV file systems. The conversion is done in the above macros for the !KERNEL case. It is done during superblock initialization in Linux ext2fs (at least the in-core copy of the superblock is written to). It is not done in FreeBSD ext2fs. Thus support for EXT2_GOOD_OLD_REV file systems has been broken. The change seems to be correct except for the above, and 1 other major style bug. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090119220943.P37158>