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:
0001-tests-add-field-sizes-to-inode-super-struct-tests.patch
Description: Binary data
