* Re: [PATCH] libext2fs: add new test: tst_inode_size
2011-09-14 22:15 ` Ted Ts'o
@ 2012-03-17 3:07 ` Andreas Dilger
2012-03-20 3:55 ` Ted Ts'o
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Dilger @ 2012-03-17 3:07 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Ext4 Developers List
[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]
On 2011-09-14, at 4:15 PM, Ted Ts'o wrote:
> On Wed, Sep 14, 2011 at 02:47:08PM -0600, Andreas Dilger wrote:
>> One thing I noticed with this check_field() macro is that it doesn't
>> actually detect the case if the size of a field is changed. This hit
>> me when I was making a cleanup to the large journal patch which renamed
>> s_jnl_blocks[15] to s_jnl_size_lo and s_jnl_blocks[16] to s_jnl_size_hi
>> for clarity. The tst_super_size test passed just fine, but the e2fsck
>> test scripts failed in weird and wonderful ways.
>>
>> A better solution might be to explicitly pass the expected field size
>> instead of getting both the size and offset from the structure itself.
>> Since these structures change very rarely it isn't much maintenance,
>> but it would be lousy if code was released that had some incorrect
>> field offset because someone increased or decreased an earlier field
>> without thinking enough, and those fields weren't used in normal tests.
>>
>> I can submit a patch if you are interested.
>
> Good point. Yes, I agree it would be worth while to do this.
Finally had a few minutes to sit down and work on this. Patch
attached, since I'm offline right now.
Cheers, Andreas
--
Andreas Dilger Whamcloud, Inc.
Principal Lustre Engineer http://www.whamcloud.com/
[-- Attachment #2: 0001-tests-add-field-sizes-to-inode-super-struct-tests.patch --]
[-- Type: application/octet-stream, Size: 12356 bytes --]
From 79b3fd2b8f4f6bf071e6373b1e58494992938589 Mon Sep 17 00:00:00 2001
From: Andreas Dilger <adilger@whamcloud.com>
Date: Thu, 15 Mar 2012 22:16:04 -0600
Subject: [PATCH] tests: add field sizes to inode/super struct tests
In addition to validating the ordering of fields within the inode
and superblock structures, also validate the field sizes. Otherwise
it is possible to incorrectly change the size of one of these fields
without getting any kind of error from these tests. Failures would
only show up later in the test image checks if the field that is
changed is before another in-use field.
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
---
lib/ext2fs/tst_inode_size.c | 97 ++++++++++----------
lib/ext2fs/tst_super_size.c | 209 +++++++++++++++++++++----------------------
2 files changed, 151 insertions(+), 155 deletions(-)
diff --git a/lib/ext2fs/tst_inode_size.c b/lib/ext2fs/tst_inode_size.c
index 86e3227..3e43d9f 100644
--- a/lib/ext2fs/tst_inode_size.c
+++ b/lib/ext2fs/tst_inode_size.c
@@ -16,67 +16,70 @@
#include "ext2_fs.h"
-struct ext2_inode inode;
+struct ext2_inode_large inode;
-int verbose = 0;
+#define offsetof(type, member) __builtin_offsetof(type, member)
+#define check_field(x, s) cur_offset = do_field(#x, s, sizeof(inode.x), \
+ offsetof(struct ext2_inode_large, x), \
+ cur_offset)
-#define offsetof(type, member) __builtin_offsetof (type, member)
-#define check_field(x) cur_offset = do_field(#x, sizeof(inode.x), \
- offsetof(struct ext2_inode, x), \
- cur_offset)
-
-static int do_field(const char *field, size_t size, int offset, int cur_offset)
+static int do_field(const char *field, unsigned size, unsigned cur_size,
+ unsigned offset, unsigned cur_offset)
{
+ if (size != cur_size) {
+ printf("error: %s size %u should be %u\n",
+ field, cur_size, size);
+ exit(1);
+ }
if (offset != cur_offset) {
- printf("Warning! Unexpected offset at %s\n", field);
+ printf("error: %s offset %u should be %u\n",
+ field, cur_offset, offset);
exit(1);
}
printf("%8d %-30s %3u\n", offset, field, (unsigned) size);
return offset + size;
}
-void check_structure_fields()
+int main(int argc, char **argv)
{
#if (__GNUC__ >= 4)
int cur_offset = 0;
printf("%8s %-30s %3s\n", "offset", "field", "size");
- check_field(i_mode);
- check_field(i_uid);
- check_field(i_size);
- check_field(i_atime);
- check_field(i_ctime);
- check_field(i_mtime);
- check_field(i_dtime);
- check_field(i_gid);
- check_field(i_links_count);
- check_field(i_blocks);
- check_field(i_flags);
- check_field(osd1.linux1.l_i_version);
- check_field(i_block);
- check_field(i_generation);
- check_field(i_file_acl);
- check_field(i_size_high);
- check_field(i_faddr);
- check_field(osd2.linux2.l_i_blocks_hi);
- check_field(osd2.linux2.l_i_file_acl_high);
- check_field(osd2.linux2.l_i_uid_high);
- check_field(osd2.linux2.l_i_gid_high);
- check_field(osd2.linux2.l_i_checksum_lo);
- check_field(osd2.linux2.l_i_reserved);
- printf("Ending offset is %d\n\n", cur_offset);
+ check_field(i_mode, 2);
+ check_field(i_uid, 2);
+ check_field(i_size, 4);
+ check_field(i_atime, 4);
+ check_field(i_ctime, 4);
+ check_field(i_mtime, 4);
+ check_field(i_dtime, 4);
+ check_field(i_gid, 2);
+ check_field(i_links_count, 2);
+ check_field(i_blocks, 4);
+ check_field(i_flags, 4);
+ check_field(osd1.linux1.l_i_version, 4);
+ check_field(i_block, 15 * 4);
+ check_field(i_generation, 4);
+ check_field(i_file_acl, 4);
+ check_field(i_size_high, 4);
+ check_field(i_faddr, 4);
+ check_field(osd2.linux2.l_i_blocks_hi, 2);
+ check_field(osd2.linux2.l_i_file_acl_high, 2);
+ check_field(osd2.linux2.l_i_uid_high, 2);
+ check_field(osd2.linux2.l_i_gid_high, 2);
+ check_field(osd2.linux2.l_i_checksum_lo, 2);
+ check_field(osd2.linux2.l_i_reserved, 2);
+ do_field("Small inode end", 0, 0, cur_offset, 128);
+ check_field(i_extra_isize, 2);
+ check_field(i_checksum_hi, 2);
+ check_field(i_ctime_extra, 4);
+ check_field(i_mtime_extra, 4);
+ check_field(i_atime_extra, 4);
+ check_field(i_crtime, 4);
+ check_field(i_crtime_extra, 4);
+ check_field(i_version_hi, 4);
+ /* This size will change as new fields are added */
+ do_field("Large inode end", 0, 0, cur_offset, sizeof(inode));
#endif
-}
-
-
-int main(int argc, char **argv)
-{
- int l = sizeof(struct ext2_inode);
-
- check_structure_fields();
- printf("Size of struct ext2_inode is %d\n", l);
- if (l != 128) {
- exit(1);
- }
- exit(0);
+ return 0;
}
diff --git a/lib/ext2fs/tst_super_size.c b/lib/ext2fs/tst_super_size.c
index 6593652..76e6e6f 100644
--- a/lib/ext2fs/tst_super_size.c
+++ b/lib/ext2fs/tst_super_size.c
@@ -21,127 +21,120 @@
struct sb_struct sb;
-int verbose = 0;
-
#define offsetof(type, member) __builtin_offsetof (type, member)
-#define check_field(x) cur_offset = do_field(#x, sizeof(sb.x), \
- offsetof(struct sb_struct, x), \
- cur_offset)
+#define check_field(x, s) cur_offset = do_field(#x, s, sizeof(sb.x), \
+ offsetof(struct sb_struct, x), \
+ cur_offset)
-static int do_field(const char *field, size_t size, int offset, int cur_offset)
+static int do_field(const char *field, unsigned size, unsigned cur_size,
+ unsigned offset, unsigned cur_offset)
{
+ if (size != cur_size) {
+ printf("error: %s size %u should be %u\n",
+ field, cur_size, size);
+ exit(1);
+ }
if (offset != cur_offset) {
- printf("Warning! Unexpected offset at %s\n", field);
+ printf("error: %s offset %u should be %u\n",
+ field, cur_offset, offset);
exit(1);
}
- printf("%8d %-30s %3u\n", offset, field, (unsigned) size);
+ printf("%8d %-30s %3u\n", offset, field, size);
return offset + size;
}
-void check_superblock_fields()
+int main(int argc, char **argv)
{
#if (__GNUC__ >= 4)
int cur_offset = 0;
printf("%8s %-30s %3s\n", "offset", "field", "size");
- check_field(s_inodes_count);
- check_field(s_blocks_count);
- check_field(s_r_blocks_count);
- check_field(s_free_blocks_count);
- check_field(s_free_inodes_count);
- check_field(s_first_data_block);
- check_field(s_log_block_size);
- check_field(s_log_cluster_size);
- check_field(s_blocks_per_group);
- check_field(s_clusters_per_group);
- check_field(s_inodes_per_group);
- check_field(s_mtime);
- check_field(s_wtime);
- check_field(s_mnt_count);
- check_field(s_max_mnt_count);
- check_field(s_magic);
- check_field(s_state);
- check_field(s_errors);
- check_field(s_minor_rev_level);
- check_field(s_lastcheck);
- check_field(s_checkinterval);
- check_field(s_creator_os);
- check_field(s_rev_level);
- check_field(s_def_resuid);
- check_field(s_def_resgid);
- check_field(s_first_ino);
- check_field(s_inode_size);
- check_field(s_block_group_nr);
- check_field(s_feature_compat);
- check_field(s_feature_incompat);
- check_field(s_feature_ro_compat);
- check_field(s_uuid);
- check_field(s_volume_name);
- check_field(s_last_mounted);
- check_field(s_algorithm_usage_bitmap);
- check_field(s_prealloc_blocks);
- check_field(s_prealloc_dir_blocks);
- check_field(s_reserved_gdt_blocks);
- check_field(s_journal_uuid);
- check_field(s_journal_inum);
- check_field(s_journal_dev);
- check_field(s_last_orphan);
- check_field(s_hash_seed);
- check_field(s_def_hash_version);
- check_field(s_jnl_backup_type);
- check_field(s_desc_size);
- check_field(s_default_mount_opts);
- check_field(s_first_meta_bg);
- check_field(s_mkfs_time);
- check_field(s_jnl_blocks);
- check_field(s_blocks_count_hi);
- check_field(s_r_blocks_count_hi);
- check_field(s_free_blocks_hi);
- check_field(s_min_extra_isize);
- check_field(s_want_extra_isize);
- check_field(s_flags);
- check_field(s_raid_stride);
- check_field(s_mmp_update_interval);
- check_field(s_mmp_block);
- check_field(s_raid_stripe_width);
- check_field(s_log_groups_per_flex);
- check_field(s_reserved_char_pad);
- check_field(s_reserved_pad);
- check_field(s_kbytes_written);
- check_field(s_snapshot_inum);
- check_field(s_snapshot_id);
- check_field(s_snapshot_r_blocks_count);
- check_field(s_snapshot_list);
- check_field(s_error_count);
- check_field(s_first_error_time);
- check_field(s_first_error_ino);
- check_field(s_first_error_block);
- check_field(s_first_error_func);
- check_field(s_first_error_line);
- check_field(s_last_error_time);
- check_field(s_last_error_ino);
- check_field(s_last_error_line);
- check_field(s_last_error_block);
- check_field(s_last_error_func);
- check_field(s_mount_opts);
- check_field(s_usr_quota_inum);
- check_field(s_grp_quota_inum);
- check_field(s_overhead_blocks);
- check_field(s_reserved);
- check_field(s_checksum);
- printf("Ending offset is %d\n\n", cur_offset);
+ check_field(s_inodes_count, 4);
+ check_field(s_blocks_count, 4);
+ check_field(s_r_blocks_count, 4);
+ check_field(s_free_blocks_count, 4);
+ check_field(s_free_inodes_count, 4);
+ check_field(s_first_data_block, 4);
+ check_field(s_log_block_size, 4);
+ check_field(s_log_cluster_size, 4);
+ check_field(s_blocks_per_group, 4);
+ check_field(s_clusters_per_group, 4);
+ check_field(s_inodes_per_group, 4);
+ check_field(s_mtime, 4);
+ check_field(s_wtime, 4);
+ check_field(s_mnt_count, 2);
+ check_field(s_max_mnt_count, 2);
+ check_field(s_magic, 2);
+ check_field(s_state, 2);
+ check_field(s_errors, 2);
+ check_field(s_minor_rev_level, 2);
+ check_field(s_lastcheck, 4);
+ check_field(s_checkinterval, 4);
+ check_field(s_creator_os, 4);
+ check_field(s_rev_level, 4);
+ check_field(s_def_resuid, 2);
+ check_field(s_def_resgid, 2);
+ check_field(s_first_ino, 4);
+ check_field(s_inode_size, 2);
+ check_field(s_block_group_nr, 2);
+ check_field(s_feature_compat, 4);
+ check_field(s_feature_incompat, 4);
+ check_field(s_feature_ro_compat, 4);
+ check_field(s_uuid, 16);
+ check_field(s_volume_name, 16);
+ check_field(s_last_mounted, 64);
+ check_field(s_algorithm_usage_bitmap, 4);
+ check_field(s_prealloc_blocks, 1);
+ check_field(s_prealloc_dir_blocks, 1);
+ check_field(s_reserved_gdt_blocks, 2);
+ check_field(s_journal_uuid, 16);
+ check_field(s_journal_inum, 4);
+ check_field(s_journal_dev, 4);
+ check_field(s_last_orphan, 4);
+ check_field(s_hash_seed, 4 * 4);
+ check_field(s_def_hash_version, 1);
+ check_field(s_jnl_backup_type, 1);
+ check_field(s_desc_size, 2);
+ check_field(s_default_mount_opts, 4);
+ check_field(s_first_meta_bg, 4);
+ check_field(s_mkfs_time, 4);
+ check_field(s_jnl_blocks, 17 * 4);
+ check_field(s_blocks_count_hi, 4);
+ check_field(s_r_blocks_count_hi, 4);
+ check_field(s_free_blocks_hi, 4);
+ check_field(s_min_extra_isize, 2);
+ check_field(s_want_extra_isize, 2);
+ check_field(s_flags, 4);
+ check_field(s_raid_stride, 2);
+ check_field(s_mmp_update_interval, 2);
+ check_field(s_mmp_block, 8);
+ check_field(s_raid_stripe_width, 4);
+ check_field(s_log_groups_per_flex, 1);
+ check_field(s_reserved_char_pad, 1);
+ check_field(s_reserved_pad, 2);
+ check_field(s_kbytes_written, 8);
+ check_field(s_snapshot_inum, 4);
+ check_field(s_snapshot_id, 4);
+ check_field(s_snapshot_r_blocks_count, 8);
+ check_field(s_snapshot_list, 4);
+ check_field(s_error_count, 4);
+ check_field(s_first_error_time, 4);
+ check_field(s_first_error_ino, 4);
+ check_field(s_first_error_block, 8);
+ check_field(s_first_error_func, 32);
+ check_field(s_first_error_line, 4);
+ check_field(s_last_error_time, 4);
+ check_field(s_last_error_ino, 4);
+ check_field(s_last_error_line, 4);
+ check_field(s_last_error_block, 8);
+ check_field(s_last_error_func, 32);
+ check_field(s_mount_opts, 64);
+ check_field(s_usr_quota_inum, 4);
+ check_field(s_grp_quota_inum, 4);
+ check_field(s_overhead_blocks, 4);
+ check_field(s_reserved, 108 * 4);
+ check_field(s_checksum, 4);
+ do_field("Superblock end", 0, 0, cur_offset, 1024);
#endif
-}
-
-
-int main(int argc, char **argv)
-{
- int s = sizeof(struct sb_struct);
-
- check_superblock_fields();
- printf("Size of struct %s is %d\n", sb_struct_name, s);
- if (s != 1024) {
- exit(1);
- }
- exit(0);
+ return 0;
}
--
1.7.2
^ permalink raw reply related [flat|nested] 5+ messages in thread