Skip to content

Commit

Permalink
ext4: ensure Inode flags consistency are checked at build time
Browse files Browse the repository at this point in the history
Flags being used by atomic operations in inode flags (e.g.
ext4_test_inode_flag(), should be consistent with that actually stored
in inodes, i.e.: EXT4_XXX_FL.

It ensures that this consistency is checked at build-time, not at
run-time.

Currently, the flags consistency are being checked at run-time, but,
there is no real reason to not do a build-time check instead of a
run-time check. The code is comparing macro defined values with enum
type variables, where both are constants, so, there is no problem in
comparing constants at build-time.

enum variables are treated as constants by the C compiler, according
to the C99 specs (see www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf 
sec. 6.2.5, item 16), so, there is no real problem in comparing an
enumeration type at build time

Signed-off-by: Carlos Maiolino <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
  • Loading branch information
cmaiolino authored and tytso committed Dec 10, 2012
1 parent 939da10 commit 9a4c801
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 16 deletions.
29 changes: 13 additions & 16 deletions fs/ext4/ext4.h
Original file line number Diff line number Diff line change
Expand Up @@ -463,25 +463,22 @@ enum {
EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
};

#define TEST_FLAG_VALUE(FLAG) (EXT4_##FLAG##_FL == (1 << EXT4_INODE_##FLAG))
#define CHECK_FLAG_VALUE(FLAG) if (!TEST_FLAG_VALUE(FLAG)) { \
printk(KERN_EMERG "EXT4 flag fail: " #FLAG ": %d %d\n", \
EXT4_##FLAG##_FL, EXT4_INODE_##FLAG); BUG_ON(1); }

/*
* Since it's pretty easy to mix up bit numbers and hex values, and we
* can't do a compile-time test for ENUM values, we use a run-time
* test to make sure that EXT4_XXX_FL is consistent with respect to
* EXT4_INODE_XXX. If all is well the printk and BUG_ON will all drop
* out so it won't cost any extra space in the compiled kernel image.
* But it's important that these values are the same, since we are
* using EXT4_INODE_XXX to test for the flag values, but EXT4_XX_FL
* must be consistent with the values of FS_XXX_FL defined in
* include/linux/fs.h and the on-disk values found in ext2, ext3, and
* ext4 filesystems, and of course the values defined in e2fsprogs.
/*
* Since it's pretty easy to mix up bit numbers and hex values, we use a
* build-time check to make sure that EXT4_XXX_FL is consistent with respect to
* EXT4_INODE_XXX. If all is well, the macros will be dropped, so, it won't cost
* any extra space in the compiled kernel image, otherwise, the build will fail.
* It's important that these values are the same, since we are using
* EXT4_INODE_XXX to test for flag values, but EXT4_XXX_FL must be consistent
* with the values of FS_XXX_FL defined in include/linux/fs.h and the on-disk
* values found in ext2, ext3 and ext4 filesystems, and of course the values
* defined in e2fsprogs.
*
* It's not paranoia if the Murphy's Law really *is* out to get you. :-)
*/
#define TEST_FLAG_VALUE(FLAG) (EXT4_##FLAG##_FL == (1 << EXT4_INODE_##FLAG))
#define CHECK_FLAG_VALUE(FLAG) BUILD_BUG_ON(!TEST_FLAG_VALUE(FLAG))

static inline void ext4_check_flag_values(void)
{
CHECK_FLAG_VALUE(SECRM);
Expand Down
1 change: 1 addition & 0 deletions fs/ext4/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -5278,6 +5278,7 @@ static int __init ext4_init_fs(void)
ext4_li_info = NULL;
mutex_init(&ext4_li_mtx);

/* Build-time check for flags consistency */
ext4_check_flag_values();

for (i = 0; i < EXT4_WQ_HASH_SZ; i++) {
Expand Down

0 comments on commit 9a4c801

Please sign in to comment.