btrfs-progs: fix endian bugs and clean-ups
Subsystems (2)
v2 cover btrfs-progs: fix endian bugs and clean-ups
This is a patch-set after getting running sparse.
There are two endian patches one is new and the other is Zach Brown's work but we lost. And rest are just for clean-ups.
v2:
- cover-letter:
add, 'btrfs-progs' prefix in subject
- btrfs-progs: fix using on-disk structure to store in memory data:
remove, "Signed-off-by: Zach Brown <[email protected]>" which was for below patch but added by mistake.
'[PATCH 4/6] btrfs-progs: fix endian bugs in chunk rebuilding'
Byongho Lee (6):
btrfs-progs: get sparse checking working
btrfs-progs: use NULL instead of 0
btrfs-progs: make private symbols to static
btrfs-progs: fix endian bugs in chunk rebuilding
btrfs-progs: fix endian bug in update_super()
btrfs-progs: fix using on-disk structure to store in memory data
Makefile.in | 2 +- btrfs-image.c | 3 +-- btrfs-show-super.c | 4 ++-- chunk-recover.c | 8 ++++---- cmds-fi-usage.c | 12 ++++++------ props.c | 2 +- qgroup-verify.c | 36 +++++++++++++++++++++--------------- qgroup.c | 6 ++++-- utils.c | 4 ++-- 9 files changed, 42 insertions(+), 35 deletions(-)
Signature · 1 line
2.6.4
PATCH v2 1/6 btrfs-progs: get sparse checking working
When I run sparse checking it gives the following error:
$ make C=1 V=1
gcc -MM -MG -MF cmds-fi-usage.o.d -MT cmds-fi-usage.o -MT \
cmds-fi-usage.static.o -MT cmds-fi-usage.o.d -g -O1 -Wall \
-D_FORTIFY_SOURCE=2 -include config.h -DBTRFS_FLAT_INCLUDES \
-D_XOPEN_SOURCE=700 -fno-strict-aliasing -fPIC cmds-fi-usage.c
[SP] ctree.c
sparse -g -O1 -Wall -D_FORTIFY_SOURCE=2 -include config.h \
-DBTRFS_FLAT_INCLUDES -D_XOPEN_SOURCE=700 -fno-strict-aliasing -fPIC \
-include -D__CHECKER__ -D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized \
-Wshadow -Wundef -U_FORTIFY_SOURCE ctree.c
builtin:1:15: error: unable to open '-D__CHECKER__'
Makefile:177: recipe for target 'ctree.o' failed
make: *** [ctree.o] Error 1It means '$(check_defs)' is passed to sparse as NULL and looks '$(check_defs)'should be assigned before assigning '$(CHECKER_FLAGS)'. BTW, I'm not familiar with make tool so there could be some my misunderstanding and better solution.
--- Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile.in +1 -1
@@ -60,6 +60,7 @@ STATIC_LIBS = @UUID_LIBS_STATIC@ @BLKID_LIBS_STATIC@ \ # generate so many sparse errors that sparse stops parsing, # which masks real errors that we want to see. CHECKER := sparse +check_defs := .cc-defines.h CHECKER_FLAGS := -include $(check_defs) -D__CHECKER__ \ -D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized -Wshadow -Wundef \ -U_FORTIFY_SOURCE
@@ -154,7 +155,6 @@ lib_links = libbtrfs.so.0 libbtrfs.so headers = $(libbtrfs_headers) # make C=1 to enable sparse -check_defs := .cc-defines.h ifdef C # We're trying to use sparse against glibc headers which go wild # trying to use internal compiler macros to test features. We
Signature · 1 line
2.6.4
PATCH v2 2/6 btrfs-progs: use NULL instead of 0
In reply to Byongho LeeFix the code assigning 0 to pointer instead of NULL.
--- cmds-fi-usage.c | 12 ++++++------ props.c | 2 +- qgroup.c | 6 ++++-- utils.c | 4 ++-- 4 files changed, 13 insertions(+), 11 deletions(-)
cmds-fi-usage.c +6 -6
@@ -47,7 +47,7 @@ static int add_info_to_list(struct chunk_info **info_ptr,for (j =0 ; j < num_stripes ; j++) {int i; -struct chunk_info *p =0 ; +struct chunk_info *p =NULL ;struct btrfs_stripe *stripe;u64 devid;
@@ -182,7 +182,7 @@ static int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count ret =add_info_to_list (info_ptr, info_count, item);if (ret) { - *info_ptr =0 ; + *info_ptr =NULL ;return 1 ; }
@@ -228,7 +228,7 @@ static int cmp_btrfs_ioctl_space_info(const void *a, const void *b)*/ static struct btrfs_ioctl_space_args *load_space_info (int fd,char *path) { -struct btrfs_ioctl_space_args *sargs =0 , *sargs_orig =0 ; +struct btrfs_ioctl_space_args *sargs =NULL , *sargs_orig =NULL ;int e, ret, count; sargs_orig = sargs =calloc (1 ,sizeof (struct btrfs_ioctl_space_args ));
@@ -312,7 +312,7 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo, int chunkcount, struct device_info *devinfo, int devcount, char *path, unsigned unit_mode) { - struct btrfs_ioctl_space_args *sargs = 0; + struct btrfs_ioctl_space_args *sargs = NULL;int i;int ret =0 ;int width =10 ;/* default 10 for human units */
@@ -510,7 +510,7 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,struct device_info *info; *device_info_count =0 ; - *device_info_ptr =0 ; + *device_info_ptr =NULL ; ret =ioctl (fd,BTRFS_IOC_FS_INFO , &fi_args);if (ret <0 ) {
@@ -620,7 +620,7 @@ static void _cmd_filesystem_usage_tabular(unsigned unit_mode, {int i;u64 total_unused =0 ; -struct string_table *matrix =0 ; +struct string_table *matrix =NULL ;int ncols, nrows;int col;int unallocated_col;
props.c +1 -1
@@ -194,5 +194,5 @@ const struct prop_handler prop_handlers[] = { prop_object_dev | prop_object_root, prop_label}, {"compression" ,"Set/get compression for a file or directory" ,0 , prop_object_inode, prop_compression}, - {0, 0, 0, 0, 0} + {NULL, NULL, 0, 0, NULL} };
qgroup.c +4 -2
@@ -1117,7 +1117,8 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)btrfs_stack_qgroup_info_exclusive_compressed (info);add_qgroup (qgroup_lookup, sh->offset, a1, a2, - a3, a4, a5,0 ,0 ,0 ,0 ,0 ,0 ,0 ); + a3, a4, a5,0 ,0 ,0 ,0 ,0 , +NULL ,NULL ); }else if (sh->type ==BTRFS_QGROUP_LIMIT_KEY ) { limit = (struct btrfs_qgroup_limit_item *) (args.buf + off);
@@ -1132,7 +1133,8 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup) a5 =btrfs_stack_qgroup_limit_rsv_exclusive (limit);add_qgroup (qgroup_lookup, sh->offset,0 ,0 , -0 ,0 ,0 , a1, a2, a3, a4, a5,0 ,0 ); +0 ,0 ,0 , a1, a2, a3, a4, a5, +NULL ,NULL ); }else if (sh->type ==BTRFS_QGROUP_RELATION_KEY ) {if (sh->offset < sh->objectid) goto skip;
utils.c +2 -2
@@ -154,7 +154,7 @@ int test_uuid_unique(char *fs_uuid)blkid_dev dev =NULL ;blkid_cache cache =NULL ; -if (blkid_get_cache (&cache,0 ) <0 ) { +if (blkid_get_cache (&cache,NULL ) <0 ) {printf ("ERROR: lblkid cache get failed\n" );return 1 ; }
@@ -2602,7 +2602,7 @@ int btrfs_scan_lblkid(void)if (btrfs_scan_done)return 0 ; -if (blkid_get_cache (&cache,0 ) <0 ) { +if (blkid_get_cache (&cache,NULL ) <0 ) {printf ("ERROR: lblkid cache get failed\n" );return 1 ; }
Signature · 1 line
2.6.4
PATCH v2 3/6 btrfs-progs: make private symbols to static
In reply to Byongho Lee--- btrfs-show-super.c | 4 ++-- qgroup-verify.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-)
btrfs-show-super.c +2 -2
@@ -334,7 +334,7 @@ struct readable_flag_entry {#define DEF_INCOMPAT_FLAG_ENTRY (bit_name) \ {BTRFS_FEATURE_INCOMPAT_##bit_name, #bit_name} -struct readable_flag_entry incompat_flags_array[] = { +static struct readable_flag_entry incompat_flags_array[] = {DEF_INCOMPAT_FLAG_ENTRY (MIXED_BACKREF ),DEF_INCOMPAT_FLAG_ENTRY (DEFAULT_SUBVOL ),DEF_INCOMPAT_FLAG_ENTRY (MIXED_GROUPS ),
@@ -354,7 +354,7 @@ static const int incompat_flags_num = sizeof(incompat_flags_array) /#define DEF_SUPER_FLAG_ENTRY (bit_name) \ {BTRFS_SUPER_FLAG_##bit_name, #bit_name} -struct readable_flag_entry super_flags_array[] = { +static struct readable_flag_entry super_flags_array[] = {DEF_HEADER_FLAG_ENTRY (WRITTEN ),DEF_HEADER_FLAG_ENTRY (RELOC ),DEF_SUPER_FLAG_ENTRY (CHANGING_FSID ),
qgroup-verify.c +4 -4
@@ -49,12 +49,12 @@ struct qgroup_count {struct rb_node rb_node; }; -struct counts_tree { +static struct counts_tree {struct rb_root root;unsigned int num_groups; } counts = { .root =RB_ROOT }; -struct rb_root by_bytenr =RB_ROOT ; +static struct rb_root by_bytenr =RB_ROOT ;/* * List of interior tree blocks. We walk this list after loading the
@@ -68,8 +68,8 @@ struct rb_root by_bytenr = RB_ROOT;* exist further down the tree, the fact that our interior node has a * ref means we need to account anything below it to all its roots. */ -struct ulist *tree_blocks =NULL ;/* unode->val = bytenr, ->aux -* = tree_block pointer */ +static struct ulist *tree_blocks =NULL ;/* unode->val = bytenr, ->aux +* = tree_block pointer */ struct tree_block {int level;u64 num_bytes;
Signature · 1 line
2.6.4
PATCH v2 4/6 btrfs-progs: fix endian bugs in chunk rebuilding
In reply to Byongho LeeThis is a same patch as Zach Brown's but we lost so I resend it based on current code.
- 'commit 2cd95f945a61 ("fix endian bugs in chunk rebuilding")'
--- chunk-recover.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
chunk-recover.c +4 -4
@@ -1159,9 +1159,9 @@ static int __rebuild_chunk_root(struct btrfs_trans_handle *trans,if (min_devid > dev->devid) min_devid = dev->devid; } - disk_key.objectid =BTRFS_DEV_ITEMS_OBJECTID ; - disk_key.type =BTRFS_DEV_ITEM_KEY ; - disk_key.offset = min_devid; +btrfs_set_disk_key_objectid (&disk_key,BTRFS_DEV_ITEMS_OBJECTID ); +btrfs_set_disk_key_type (&disk_key,BTRFS_DEV_ITEM_KEY ); +btrfs_set_disk_key_offset (&disk_key, min_devid); cow = btrfs_alloc_free_block(trans, root, root->nodesize, BTRFS_CHUNK_TREE_OBJECTID,
@@ -1234,7 +1234,7 @@ static int __insert_chunk_item(struct btrfs_trans_handle *trans, key.offset = chunk_rec->offset; ret =btrfs_insert_item (trans, chunk_root, &key, chunk, -btrfs_chunk_item_size (chunk->num_stripes)); +btrfs_chunk_item_size (chunk_rec->num_stripes));free (chunk);return ret; }
Signature · 1 line
2.6.4
PATCH v2 5/6 btrfs-progs: fix endian bug in update_super()
In reply to Byongho LeeIn update_super() 'chunk->stripe.devid' and 'super->dev_item.devid' both are little endian. So we should not use endian helper btrfs_set_stack_stripe_devid().
--- btrfs-image.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
btrfs-image.c +1 -2
@@ -1464,8 +1464,7 @@ static int update_super(struct mdrestore_struct *mdres, u8 *buffer)btrfs_set_stack_chunk_sub_stripes (chunk,0 );btrfs_set_stack_chunk_type (chunk,BTRFS_BLOCK_GROUP_SYSTEM ); -btrfs_set_stack_stripe_devid (&chunk->stripe, - super->dev_item.devid); + chunk->stripe.devid = super->dev_item.devid; physical =logical_to_physical (mdres, key.offset, &size);if (size != (u64)-1 )
Signature · 1 line
2.6.4
PATCH v2 6/6 btrfs-progs: fix using on-disk structure to store in memory data
In reply to Byongho LeeIn 'qgroup_count' structure 'diskinfo' and 'info' are used to store only in memory data but its types are for on-disk structure as a result sparse warns it (different base types). So fix it by adding new structure 'qgroup_info' to store in memory data and replace on-disk structure 'btrfs_qgroup_info_item' by 'qgroup_info'. In addition in alloc_cnt() 'generation' is set but not used after that so remove the relevant code.
---
v2:
remove, "Signed-off-by: Zach Brown <[email protected]>" which was for below patch but added by mistake.
'[PATCH 4/6] btrfs-progs: fix endian bugs in chunk rebuilding'
--- qgroup-verify.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)
qgroup-verify.c +17 -11
@@ -37,16 +37,23 @@ static unsigned long tot_extents_scanned = 0;static void add_bytes (u64 root_objectid,u64 num_bytes,int exclusive); +struct qgroup_info { +u64 referenced; +u64 referenced_compressed; +u64 exclusive; +u64 exclusive_compressed; +}; +struct qgroup_count { -u64 qgroupid; -int subvol_exists; +u64 qgroupid; +int subvol_exists; -struct btrfs_disk_key key; -struct btrfs_qgroup_info_item diskinfo; +struct btrfs_disk_key key; +struct qgroup_info diskinfo; -struct btrfs_qgroup_info_item info; +struct qgroup_info info; -struct rb_node rb_node; +struct rb_node rb_node; }; static struct counts_tree {
@@ -647,14 +654,13 @@ static struct qgroup_count *alloc_count(struct btrfs_disk_key *key, struct btrfs_qgroup_info_item *disk) {struct qgroup_count *c =calloc (1 ,sizeof (*c)); -struct btrfs_qgroup_info_item *item; +struct qgroup_info *item;if (c) { c->qgroupid =btrfs_disk_key_offset (key); c->key = *key; item = &c->diskinfo; - item->generation =btrfs_qgroup_info_generation (leaf, disk); item->referenced =btrfs_qgroup_info_referenced (leaf, disk); item->referenced_compressed =btrfs_qgroup_info_referenced_compressed (leaf, disk);
@@ -673,7 +679,7 @@ static struct qgroup_count *alloc_count(struct btrfs_disk_key *key,static void add_bytes (u64 root_objectid,u64 num_bytes,int exclusive) {struct qgroup_count *count =find_count (root_objectid); -struct btrfs_qgroup_info_item *qg; +struct qgroup_info *qg;BUG_ON (num_bytes <4096 );/* Random sanity check. */
@@ -1014,8 +1020,8 @@ static void print_fields_signed(long long bytes,static void print_qgroup_difference (struct qgroup_count *count,int verbose) {int is_different; -struct btrfs_qgroup_info_item *info = &count->info; -struct btrfs_qgroup_info_item *disk = &count->diskinfo; +struct qgroup_info *info = &count->info; +struct qgroup_info *disk = &count->diskinfo;long long excl_diff = info->exclusive - disk->exclusive;long long ref_diff = info->referenced - disk->referenced;
Signature · 1 line
2.6.4
Quoted 8 lines · Byongho Lee
This is a patch-set after getting running sparse.
There are two endian patches one is new and the other is Zach Brown's work but we lost. And rest are just for clean-ups.
v2:
- cover-letter:
add, 'btrfs-progs' prefix in subject
JFYI, this by itself would not be a reason for a v2, the patch subjects contained 'progs'.
Quoted 5 lines
- btrfs-progs: fix using on-disk structure to store in memory data:
remove, "Signed-off-by: Zach Brown <[email protected]>" which was for below patch but added by mistake.
'[PATCH 4/6] btrfs-progs: fix endian bugs in chunk rebuilding'
Quoted 7 lines
Byongho Lee (6):
btrfs-progs: get sparse checking working
btrfs-progs: use NULL instead of 0
btrfs-progs: make private symbols to static
btrfs-progs: fix endian bugs in chunk rebuilding
btrfs-progs: fix endian bug inupdate_super()
btrfs-progs: fix using on-disk structure to store in memory data
All applied, thanks.