* [PATCH] debugfs/set_fields: fix several errors and add assertions
@ 2015-02-24 12:46 Konstantin Khlebnikov
2015-02-25 4:24 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Konstantin Khlebnikov @ 2015-02-24 12:46 UTC (permalink / raw)
To: linux-ext4, Theodore Ts'o, Darrick J. Wong
Fix copy-n-paste errors:
* remove duplicate "lastcheck" and "min_extra_isize"
* fix pointer for "first_error_line" and "last_error_line"
* remove superblock field "inodes_count" from inode fields
* add null-termination for mmp_fields
Add assertions for catching such errors in the future.
Mark true aliases with flag "FLAG_ALIAS" and suppress assert for them.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
debugfs/set_fields.c | 65 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 55 insertions(+), 10 deletions(-)
diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index 60695ad..1af7d0f 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -30,6 +30,7 @@
#ifdef HAVE_ERRNO_H
#include <errno.h>
#endif
+#include <assert.h>
#if HAVE_STRINGS_H
#include <strings.h>
#endif
@@ -50,6 +51,7 @@ static ext2_ino_t set_ino;
static int array_idx;
#define FLAG_ARRAY 0x0001
+#define FLAG_ALIAS 0x0002 /* Data intersects with other field */
struct field_set_info {
const char *name;
@@ -110,7 +112,6 @@ static struct field_set_info super_fields[] = {
{ "uuid", &set_sb.s_uuid, NULL, 16, parse_uuid },
{ "volume_name", &set_sb.s_volume_name, NULL, 16, parse_string },
{ "last_mounted", &set_sb.s_last_mounted, NULL, 64, parse_string },
- { "lastcheck", &set_sb.s_lastcheck, NULL, 4, parse_uint },
{ "algorithm_usage_bitmap", &set_sb.s_algorithm_usage_bitmap, NULL,
4, parse_uint },
{ "prealloc_blocks", &set_sb.s_prealloc_blocks, NULL, 1, parse_uint },
@@ -135,7 +136,6 @@ static struct field_set_info super_fields[] = {
{ "want_extra_isize", &set_sb.s_want_extra_isize, NULL, 2, parse_uint },
{ "flags", &set_sb.s_flags, NULL, 4, parse_uint },
{ "raid_stride", &set_sb.s_raid_stride, NULL, 2, parse_uint },
- { "min_extra_isize", &set_sb.s_min_extra_isize, NULL, 4, parse_uint },
{ "mmp_interval", &set_sb.s_mmp_update_interval, NULL, 2, parse_uint },
{ "mmp_block", &set_sb.s_mmp_block, NULL, 8, parse_uint },
{ "raid_stripe_width", &set_sb.s_raid_stripe_width, NULL, 4, parse_uint },
@@ -159,19 +159,18 @@ static struct field_set_info super_fields[] = {
{ "first_error_ino", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
{ "first_error_block", &set_sb.s_first_error_block, NULL, 8, parse_uint },
{ "first_error_func", &set_sb.s_first_error_func, NULL, 32, parse_string },
- { "first_error_line", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
+ { "first_error_line", &set_sb.s_first_error_line, NULL, 4, parse_uint },
{ "last_error_time", &set_sb.s_last_error_time, NULL, 4, parse_time },
{ "last_error_ino", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
{ "last_error_block", &set_sb.s_last_error_block, NULL, 8, parse_uint },
{ "last_error_func", &set_sb.s_last_error_func, NULL, 32, parse_string },
- { "last_error_line", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
+ { "last_error_line", &set_sb.s_last_error_line, NULL, 4, parse_uint },
{ "encrypt_algos", &set_sb.s_encrypt_algos, NULL, 1, parse_uint,
FLAG_ARRAY, 4 },
{ 0, 0, 0, 0 }
};
static struct field_set_info inode_fields[] = {
- { "inodes_count", &set_sb.s_inodes_count, NULL, 4, parse_uint },
{ "mode", &set_inode.i_mode, NULL, 2, parse_uint },
{ "uid", &set_inode.i_uid, &set_inode.osd2.linux2.l_i_uid_high,
2, parse_uint },
@@ -189,7 +188,8 @@ static struct field_set_info inode_fields[] = {
{ "flags", &set_inode.i_flags, NULL, 4, parse_uint },
{ "version", &set_inode.osd1.linux1.l_i_version,
&set_inode.i_version_hi, 4, parse_uint },
- { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL, 4, parse_uint },
+ { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL,
+ 4, parse_uint, FLAG_ALIAS },
{ "block", &set_inode.i_block[0], NULL, 4, parse_uint, FLAG_ARRAY,
EXT2_NDIR_BLOCKS },
{ "block[IND]", &set_inode.i_block[EXT2_IND_BLOCK], NULL, 4, parse_uint },
@@ -199,14 +199,14 @@ static struct field_set_info inode_fields[] = {
/* Special case: i_file_acl_high is 2 bytes */
{ "file_acl", &set_inode.i_file_acl,
&set_inode.osd2.linux2.l_i_file_acl_high, 6, parse_uint },
- { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint },
+ { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint, FLAG_ALIAS },
{ "faddr", &set_inode.i_faddr, NULL, 4, parse_uint },
- { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint },
+ { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint, FLAG_ALIAS },
{ "fsize", &set_inode.osd2.hurd2.h_i_fsize, NULL, 1, parse_uint },
{ "checksum", &set_inode.osd2.linux2.l_i_checksum_lo,
&set_inode.i_checksum_hi, 2, parse_uint },
{ "author", &set_inode.osd2.hurd2.h_i_author, NULL,
- 4, parse_uint },
+ 4, parse_uint, FLAG_ALIAS },
{ "extra_isize", &set_inode.i_extra_isize, NULL,
2, parse_uint },
{ "ctime_extra", &set_inode.i_ctime_extra, NULL,
@@ -262,7 +262,8 @@ static struct field_set_info ext4_bg_fields[] = {
};
static struct field_set_info mmp_fields[] = {
- { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp), parse_mmp_clear },
+ { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp),
+ parse_mmp_clear, FLAG_ALIAS },
{ "magic", &set_mmp.mmp_magic, NULL, 4, parse_uint },
{ "seq", &set_mmp.mmp_seq, NULL, 4, parse_uint },
{ "time", &set_mmp.mmp_time, NULL, 8, parse_uint },
@@ -272,8 +273,52 @@ static struct field_set_info mmp_fields[] = {
parse_string },
{ "check_interval", &set_mmp.mmp_check_interval, NULL, 2, parse_uint },
{ "checksum", &set_mmp.mmp_checksum, NULL, 4, parse_uint },
+ { 0, 0, 0, 0 }
};
+static void do_verify_field_set_info(struct field_set_info *fields,
+ const void *data, size_t size)
+{
+ struct field_set_info *ss, *ss2;
+ const char *begin = (char *)data;
+ const char *end = begin + size;
+
+ for (ss = fields ; ss->name ; ss++) {
+ const char *ptr;
+
+ /* Check pointers */
+ ptr = ss->ptr;
+ assert(!ptr || (ptr >= begin && ptr < end));
+ ptr = ss->ptr2;
+ assert(!ptr || (ptr >= begin && ptr < end));
+
+ /* Check function */
+ assert(ss->func);
+
+ for (ss2 = fields ; ss2 != ss ; ss2++) {
+ /* Check duplicate names */
+ assert(strcmp(ss->name, ss2->name));
+
+ if (ss->flags & FLAG_ALIAS || ss2->flags & FLAG_ALIAS)
+ continue;
+ /* Check false aliases, might be copy-n-paste error */
+ assert(!ss->ptr || (ss->ptr != ss2->ptr &&
+ ss->ptr != ss2->ptr2));
+ assert(!ss->ptr2 || (ss->ptr2 != ss2->ptr &&
+ ss->ptr2 != ss2->ptr2));
+ }
+ }
+}
+
+static __attribute__((constructor)) void verify_field_set_info(void)
+{
+ do_verify_field_set_info(super_fields, &set_sb, sizeof(set_sb));
+ do_verify_field_set_info(inode_fields, &set_inode, sizeof(set_inode));
+ do_verify_field_set_info(ext2_bg_fields, &set_gd, sizeof(set_gd));
+ do_verify_field_set_info(ext4_bg_fields, &set_gd4, sizeof(set_gd4));
+ do_verify_field_set_info(mmp_fields, &set_mmp, sizeof(set_mmp));
+}
+
static int check_suffix(const char *field)
{
int len = strlen(field);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] debugfs/set_fields: fix several errors and add assertions
2015-02-24 12:46 [PATCH] debugfs/set_fields: fix several errors and add assertions Konstantin Khlebnikov
@ 2015-02-25 4:24 ` Darrick J. Wong
2015-02-25 5:40 ` Andreas Dilger
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2015-02-25 4:24 UTC (permalink / raw)
To: Konstantin Khlebnikov; +Cc: linux-ext4, Theodore Ts'o
On Tue, Feb 24, 2015 at 03:46:11PM +0300, Konstantin Khlebnikov wrote:
> Fix copy-n-paste errors:
> * remove duplicate "lastcheck" and "min_extra_isize"
> * fix pointer for "first_error_line" and "last_error_line"
> * remove superblock field "inodes_count" from inode fields
> * add null-termination for mmp_fields
>
> Add assertions for catching such errors in the future.
> Mark true aliases with flag "FLAG_ALIAS" and suppress assert for them.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
> debugfs/set_fields.c | 65 ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
> index 60695ad..1af7d0f 100644
> --- a/debugfs/set_fields.c
> +++ b/debugfs/set_fields.c
> @@ -30,6 +30,7 @@
> #ifdef HAVE_ERRNO_H
> #include <errno.h>
> #endif
> +#include <assert.h>
> #if HAVE_STRINGS_H
> #include <strings.h>
> #endif
> @@ -50,6 +51,7 @@ static ext2_ino_t set_ino;
> static int array_idx;
>
> #define FLAG_ARRAY 0x0001
> +#define FLAG_ALIAS 0x0002 /* Data intersects with other field */
>
> struct field_set_info {
> const char *name;
> @@ -110,7 +112,6 @@ static struct field_set_info super_fields[] = {
> { "uuid", &set_sb.s_uuid, NULL, 16, parse_uuid },
> { "volume_name", &set_sb.s_volume_name, NULL, 16, parse_string },
> { "last_mounted", &set_sb.s_last_mounted, NULL, 64, parse_string },
> - { "lastcheck", &set_sb.s_lastcheck, NULL, 4, parse_uint },
> { "algorithm_usage_bitmap", &set_sb.s_algorithm_usage_bitmap, NULL,
> 4, parse_uint },
> { "prealloc_blocks", &set_sb.s_prealloc_blocks, NULL, 1, parse_uint },
> @@ -135,7 +136,6 @@ static struct field_set_info super_fields[] = {
> { "want_extra_isize", &set_sb.s_want_extra_isize, NULL, 2, parse_uint },
> { "flags", &set_sb.s_flags, NULL, 4, parse_uint },
> { "raid_stride", &set_sb.s_raid_stride, NULL, 2, parse_uint },
> - { "min_extra_isize", &set_sb.s_min_extra_isize, NULL, 4, parse_uint },
> { "mmp_interval", &set_sb.s_mmp_update_interval, NULL, 2, parse_uint },
> { "mmp_block", &set_sb.s_mmp_block, NULL, 8, parse_uint },
> { "raid_stripe_width", &set_sb.s_raid_stripe_width, NULL, 4, parse_uint },
> @@ -159,19 +159,18 @@ static struct field_set_info super_fields[] = {
> { "first_error_ino", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
> { "first_error_block", &set_sb.s_first_error_block, NULL, 8, parse_uint },
> { "first_error_func", &set_sb.s_first_error_func, NULL, 32, parse_string },
> - { "first_error_line", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
> + { "first_error_line", &set_sb.s_first_error_line, NULL, 4, parse_uint },
> { "last_error_time", &set_sb.s_last_error_time, NULL, 4, parse_time },
> { "last_error_ino", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
> { "last_error_block", &set_sb.s_last_error_block, NULL, 8, parse_uint },
> { "last_error_func", &set_sb.s_last_error_func, NULL, 32, parse_string },
> - { "last_error_line", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
> + { "last_error_line", &set_sb.s_last_error_line, NULL, 4, parse_uint },
> { "encrypt_algos", &set_sb.s_encrypt_algos, NULL, 1, parse_uint,
> FLAG_ARRAY, 4 },
> { 0, 0, 0, 0 }
> };
>
> static struct field_set_info inode_fields[] = {
> - { "inodes_count", &set_sb.s_inodes_count, NULL, 4, parse_uint },
> { "mode", &set_inode.i_mode, NULL, 2, parse_uint },
> { "uid", &set_inode.i_uid, &set_inode.osd2.linux2.l_i_uid_high,
> 2, parse_uint },
> @@ -189,7 +188,8 @@ static struct field_set_info inode_fields[] = {
> { "flags", &set_inode.i_flags, NULL, 4, parse_uint },
> { "version", &set_inode.osd1.linux1.l_i_version,
> &set_inode.i_version_hi, 4, parse_uint },
> - { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL, 4, parse_uint },
> + { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL,
> + 4, parse_uint, FLAG_ALIAS },
> { "block", &set_inode.i_block[0], NULL, 4, parse_uint, FLAG_ARRAY,
> EXT2_NDIR_BLOCKS },
> { "block[IND]", &set_inode.i_block[EXT2_IND_BLOCK], NULL, 4, parse_uint },
> @@ -199,14 +199,14 @@ static struct field_set_info inode_fields[] = {
> /* Special case: i_file_acl_high is 2 bytes */
> { "file_acl", &set_inode.i_file_acl,
> &set_inode.osd2.linux2.l_i_file_acl_high, 6, parse_uint },
> - { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint },
> + { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint, FLAG_ALIAS },
> { "faddr", &set_inode.i_faddr, NULL, 4, parse_uint },
> - { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint },
> + { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint, FLAG_ALIAS },
> { "fsize", &set_inode.osd2.hurd2.h_i_fsize, NULL, 1, parse_uint },
> { "checksum", &set_inode.osd2.linux2.l_i_checksum_lo,
> &set_inode.i_checksum_hi, 2, parse_uint },
> { "author", &set_inode.osd2.hurd2.h_i_author, NULL,
> - 4, parse_uint },
> + 4, parse_uint, FLAG_ALIAS },
> { "extra_isize", &set_inode.i_extra_isize, NULL,
> 2, parse_uint },
> { "ctime_extra", &set_inode.i_ctime_extra, NULL,
> @@ -262,7 +262,8 @@ static struct field_set_info ext4_bg_fields[] = {
> };
>
> static struct field_set_info mmp_fields[] = {
> - { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp), parse_mmp_clear },
> + { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp),
> + parse_mmp_clear, FLAG_ALIAS },
> { "magic", &set_mmp.mmp_magic, NULL, 4, parse_uint },
> { "seq", &set_mmp.mmp_seq, NULL, 4, parse_uint },
> { "time", &set_mmp.mmp_time, NULL, 8, parse_uint },
> @@ -272,8 +273,52 @@ static struct field_set_info mmp_fields[] = {
> parse_string },
> { "check_interval", &set_mmp.mmp_check_interval, NULL, 2, parse_uint },
> { "checksum", &set_mmp.mmp_checksum, NULL, 4, parse_uint },
> + { 0, 0, 0, 0 }
Looks good so far.
> };
>
> +static void do_verify_field_set_info(struct field_set_info *fields,
> + const void *data, size_t size)
> +{
> + struct field_set_info *ss, *ss2;
> + const char *begin = (char *)data;
> + const char *end = begin + size;
> +
> + for (ss = fields ; ss->name ; ss++) {
> + const char *ptr;
> +
> + /* Check pointers */
> + ptr = ss->ptr;
> + assert(!ptr || (ptr >= begin && ptr < end));
> + ptr = ss->ptr2;
> + assert(!ptr || (ptr >= begin && ptr < end));
> +
> + /* Check function */
> + assert(ss->func);
> +
> + for (ss2 = fields ; ss2 != ss ; ss2++) {
> + /* Check duplicate names */
> + assert(strcmp(ss->name, ss2->name));
> +
> + if (ss->flags & FLAG_ALIAS || ss2->flags & FLAG_ALIAS)
> + continue;
> + /* Check false aliases, might be copy-n-paste error */
> + assert(!ss->ptr || (ss->ptr != ss2->ptr &&
> + ss->ptr != ss2->ptr2));
> + assert(!ss->ptr2 || (ss->ptr2 != ss2->ptr &&
> + ss->ptr2 != ss2->ptr2));
> + }
> + }
> +}
> +
> +static __attribute__((constructor)) void verify_field_set_info(void)
> +{
> + do_verify_field_set_info(super_fields, &set_sb, sizeof(set_sb));
> + do_verify_field_set_info(inode_fields, &set_inode, sizeof(set_inode));
> + do_verify_field_set_info(ext2_bg_fields, &set_gd, sizeof(set_gd));
> + do_verify_field_set_info(ext4_bg_fields, &set_gd4, sizeof(set_gd4));
> + do_verify_field_set_info(mmp_fields, &set_mmp, sizeof(set_mmp));
> +}
This ought to be run along with the 'make check' testcases, since they're
already looking for errors there.
Also, does running this on /every/ debugfs invocation slow down startup
noticeably? Just idle curiosity. :)
--D
> +
> static int check_suffix(const char *field)
> {
> int len = strlen(field);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] debugfs/set_fields: fix several errors and add assertions
2015-02-25 4:24 ` Darrick J. Wong
@ 2015-02-25 5:40 ` Andreas Dilger
0 siblings, 0 replies; 3+ messages in thread
From: Andreas Dilger @ 2015-02-25 5:40 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Darrick J. Wong, Ext4 Developers List, Theodore Ts'o
On Feb 24, 2015, at 9:24 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Tue, Feb 24, 2015 at 03:46:11PM +0300, Konstantin Khlebnikov wrote:
>> Fix copy-n-paste errors:
>> * remove duplicate "lastcheck" and "min_extra_isize"
>> * fix pointer for "first_error_line" and "last_error_line"
>> * remove superblock field "inodes_count" from inode fields
>> * add null-termination for mmp_fields
>>
>> Add assertions for catching such errors in the future.
>> Mark true aliases with flag "FLAG_ALIAS" and suppress assert for them.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> ---
>> debugfs/set_fields.c | 65 ++++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 55 insertions(+), 10 deletions(-)
>>
>> diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
>> index 60695ad..1af7d0f 100644
>> --- a/debugfs/set_fields.c
>> +++ b/debugfs/set_fields.c
>> @@ -30,6 +30,7 @@
>> #ifdef HAVE_ERRNO_H
>> #include <errno.h>
>> #endif
>> +#include <assert.h>
>> #if HAVE_STRINGS_H
>> #include <strings.h>
>> #endif
>> @@ -50,6 +51,7 @@ static ext2_ino_t set_ino;
>> static int array_idx;
>>
>> #define FLAG_ARRAY 0x0001
>> +#define FLAG_ALIAS 0x0002 /* Data intersects with other field */
>>
>> struct field_set_info {
>> const char *name;
>> @@ -110,7 +112,6 @@ static struct field_set_info super_fields[] = {
>> { "uuid", &set_sb.s_uuid, NULL, 16, parse_uuid },
>> { "volume_name", &set_sb.s_volume_name, NULL, 16, parse_string },
>> { "last_mounted", &set_sb.s_last_mounted, NULL, 64, parse_string },
>> - { "lastcheck", &set_sb.s_lastcheck, NULL, 4, parse_uint },
>> { "algorithm_usage_bitmap", &set_sb.s_algorithm_usage_bitmap, NULL,
>> 4, parse_uint },
>> { "prealloc_blocks", &set_sb.s_prealloc_blocks, NULL, 1, parse_uint },
>> @@ -135,7 +136,6 @@ static struct field_set_info super_fields[] = {
>> { "want_extra_isize", &set_sb.s_want_extra_isize, NULL, 2, parse_uint },
>> { "flags", &set_sb.s_flags, NULL, 4, parse_uint },
>> { "raid_stride", &set_sb.s_raid_stride, NULL, 2, parse_uint },
>> - { "min_extra_isize", &set_sb.s_min_extra_isize, NULL, 4, parse_uint },
>> { "mmp_interval", &set_sb.s_mmp_update_interval, NULL, 2, parse_uint },
>> { "mmp_block", &set_sb.s_mmp_block, NULL, 8, parse_uint },
>> { "raid_stripe_width", &set_sb.s_raid_stripe_width, NULL, 4, parse_uint },
>> @@ -159,19 +159,18 @@ static struct field_set_info super_fields[] = {
>> { "first_error_ino", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
>> { "first_error_block", &set_sb.s_first_error_block, NULL, 8, parse_uint },
>> { "first_error_func", &set_sb.s_first_error_func, NULL, 32, parse_string },
>> - { "first_error_line", &set_sb.s_first_error_ino, NULL, 4, parse_uint },
>> + { "first_error_line", &set_sb.s_first_error_line, NULL, 4, parse_uint },
>> { "last_error_time", &set_sb.s_last_error_time, NULL, 4, parse_time },
>> { "last_error_ino", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
>> { "last_error_block", &set_sb.s_last_error_block, NULL, 8, parse_uint },
>> { "last_error_func", &set_sb.s_last_error_func, NULL, 32, parse_string },
>> - { "last_error_line", &set_sb.s_last_error_ino, NULL, 4, parse_uint },
>> + { "last_error_line", &set_sb.s_last_error_line, NULL, 4, parse_uint },
>> { "encrypt_algos", &set_sb.s_encrypt_algos, NULL, 1, parse_uint,
>> FLAG_ARRAY, 4 },
>> { 0, 0, 0, 0 }
>> };
>>
>> static struct field_set_info inode_fields[] = {
>> - { "inodes_count", &set_sb.s_inodes_count, NULL, 4, parse_uint },
>> { "mode", &set_inode.i_mode, NULL, 2, parse_uint },
>> { "uid", &set_inode.i_uid, &set_inode.osd2.linux2.l_i_uid_high,
>> 2, parse_uint },
>> @@ -189,7 +188,8 @@ static struct field_set_info inode_fields[] = {
>> { "flags", &set_inode.i_flags, NULL, 4, parse_uint },
>> { "version", &set_inode.osd1.linux1.l_i_version,
>> &set_inode.i_version_hi, 4, parse_uint },
>> - { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL, 4, parse_uint },
>> + { "translator", &set_inode.osd1.hurd1.h_i_translator, NULL,
>> + 4, parse_uint, FLAG_ALIAS },
>> { "block", &set_inode.i_block[0], NULL, 4, parse_uint, FLAG_ARRAY,
>> EXT2_NDIR_BLOCKS },
>> { "block[IND]", &set_inode.i_block[EXT2_IND_BLOCK], NULL, 4, parse_uint },
>> @@ -199,14 +199,14 @@ static struct field_set_info inode_fields[] = {
>> /* Special case: i_file_acl_high is 2 bytes */
>> { "file_acl", &set_inode.i_file_acl,
>> &set_inode.osd2.linux2.l_i_file_acl_high, 6, parse_uint },
>> - { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint },
>> + { "dir_acl", &set_inode.i_dir_acl, NULL, 4, parse_uint, FLAG_ALIAS },
>> { "faddr", &set_inode.i_faddr, NULL, 4, parse_uint },
>> - { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint },
>> + { "frag", &set_inode.osd2.hurd2.h_i_frag, NULL, 1, parse_uint, FLAG_ALIAS },
>> { "fsize", &set_inode.osd2.hurd2.h_i_fsize, NULL, 1, parse_uint },
>> { "checksum", &set_inode.osd2.linux2.l_i_checksum_lo,
>> &set_inode.i_checksum_hi, 2, parse_uint },
>> { "author", &set_inode.osd2.hurd2.h_i_author, NULL,
>> - 4, parse_uint },
>> + 4, parse_uint, FLAG_ALIAS },
>> { "extra_isize", &set_inode.i_extra_isize, NULL,
>> 2, parse_uint },
>> { "ctime_extra", &set_inode.i_ctime_extra, NULL,
>> @@ -262,7 +262,8 @@ static struct field_set_info ext4_bg_fields[] = {
>> };
>>
>> static struct field_set_info mmp_fields[] = {
>> - { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp), parse_mmp_clear },
>> + { "clear", &set_mmp.mmp_magic, NULL, sizeof(set_mmp),
>> + parse_mmp_clear, FLAG_ALIAS },
>> { "magic", &set_mmp.mmp_magic, NULL, 4, parse_uint },
>> { "seq", &set_mmp.mmp_seq, NULL, 4, parse_uint },
>> { "time", &set_mmp.mmp_time, NULL, 8, parse_uint },
>> @@ -272,8 +273,52 @@ static struct field_set_info mmp_fields[] = {
>> parse_string },
>> { "check_interval", &set_mmp.mmp_check_interval, NULL, 2, parse_uint },
>> { "checksum", &set_mmp.mmp_checksum, NULL, 4, parse_uint },
>> + { 0, 0, 0, 0 }
>
> Looks good so far.
>
>> };
>>
>> +static void do_verify_field_set_info(struct field_set_info *fields,
>> + const void *data, size_t size)
>> +{
>> + struct field_set_info *ss, *ss2;
>> + const char *begin = (char *)data;
>> + const char *end = begin + size;
>> +
>> + for (ss = fields ; ss->name ; ss++) {
>> + const char *ptr;
>> +
>> + /* Check pointers */
>> + ptr = ss->ptr;
>> + assert(!ptr || (ptr >= begin && ptr < end));
>> + ptr = ss->ptr2;
>> + assert(!ptr || (ptr >= begin && ptr < end));
>> +
>> + /* Check function */
>> + assert(ss->func);
>> +
>> + for (ss2 = fields ; ss2 != ss ; ss2++) {
>> + /* Check duplicate names */
>> + assert(strcmp(ss->name, ss2->name));
>> +
>> + if (ss->flags & FLAG_ALIAS || ss2->flags & FLAG_ALIAS)
>> + continue;
>> + /* Check false aliases, might be copy-n-paste error */
>> + assert(!ss->ptr || (ss->ptr != ss2->ptr &&
>> + ss->ptr != ss2->ptr2));
>> + assert(!ss->ptr2 || (ss->ptr2 != ss2->ptr &&
>> + ss->ptr2 != ss2->ptr2));
>> + }
>> + }
>> +}
>> +
>> +static __attribute__((constructor)) void verify_field_set_info(void)
>> +{
>> + do_verify_field_set_info(super_fields, &set_sb, sizeof(set_sb));
>> + do_verify_field_set_info(inode_fields, &set_inode, sizeof(set_inode));
>> + do_verify_field_set_info(ext2_bg_fields, &set_gd, sizeof(set_gd));
>> + do_verify_field_set_info(ext4_bg_fields, &set_gd4, sizeof(set_gd4));
>> + do_verify_field_set_info(mmp_fields, &set_mmp, sizeof(set_mmp));
>> +}
>
> This ought to be run along with the 'make check' testcases, since they're
> already looking for errors there.
>
> Also, does running this on /every/ debugfs invocation slow down startup
> noticeably? Just idle curiosity. :)
Many sources in e2fsprogs have compile-time test code that can be
used to test something like this. They are enabled under "#ifdef DEBUG",
for example lib/ext2fs/icount.c is built as "tst_icount" with -DDEBUG
in order to test its functionality with "make check" rather than running
this test for every normal invocation:
tst_icount: $(srcdir)/icount.c $(STATIC_LIBEXT2FS) $(DEPSTATIC_LIBCOM_ERR)
$(E) " LD $@"
$(Q) $(CC) -o tst_icount $(srcdir)/icount.c -DDEBUG $(ALL_CFLAGS) \
$(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) $(SYSLIBS)
In e2fsck/problem.c is a similar functionality under "#ifdef UNITTEST" to
build the tst_problem binary for "make check" to verify that there are no
duplicate problem codes defined.
I'm not sure if Ted has a preference between DEBUG and UNITTEST, but that
is definitely preferable to running this for every debugfs invocation.
Cheers, Andreas
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-25 5:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-24 12:46 [PATCH] debugfs/set_fields: fix several errors and add assertions Konstantin Khlebnikov
2015-02-25 4:24 ` Darrick J. Wong
2015-02-25 5:40 ` Andreas Dilger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox