[PATCH v2 0/6] btrfs-progs: fix endian bugs and clean-ups

[PATCH v2 0/6] btrfs-progs: fix endian bugs and clean-ups

[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
 - btrfs-progs: fix using on-disk structure to store in memory data:
    remove, "Signed-off-by: Zach Brown <zab@redhat.com>" 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(-)

-- 
2.6.4

[PATCH v2 1/6] btrfs-progs: get sparse checking working

[Byongho Lee]

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 1

It 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.

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index 85b45e5bee3b..3b98709f31a5 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -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
-- 
2.6.4

[PATCH v2 2/6] btrfs-progs: use NULL instead of 0

[Byongho Lee]

Fix the code assigning 0 to pointer instead of NULL.

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 cmds-fi-usage.c | 12 ++++++------
 props.c         |  2 +-
 qgroup.c        |  6 ++++--
 utils.c         |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index 72d80278e259..852e5651db8c 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -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;
diff --git a/props.c b/props.c
index c9e2bd4108f8..5b7493240b09 100644
--- a/props.c
+++ b/props.c
@@ -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}
 };
diff --git a/qgroup.c b/qgroup.c
index 1fbfcb97d659..f56f51a0e4aa 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -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;
diff --git a/utils.c b/utils.c
index d5f60a420135..76a2bf4aa2a8 100644
--- a/utils.c
+++ b/utils.c
@@ -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;
 	}
-- 
2.6.4

[PATCH v2 3/6] btrfs-progs: make private symbols to static

[Byongho Lee]

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 btrfs-show-super.c | 4 ++--
 qgroup-verify.c    | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/btrfs-show-super.c b/btrfs-show-super.c
index c0ffeacb928c..f11701539d1a 100644
--- a/btrfs-show-super.c
+++ b/btrfs-show-super.c
@@ -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),
diff --git a/qgroup-verify.c b/qgroup-verify.c
index f7a94bfb0d32..0ee52ff5c857 100644
--- a/qgroup-verify.c
+++ b/qgroup-verify.c
@@ -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;
-- 
2.6.4

[PATCH v2 4/6] btrfs-progs: fix endian bugs in chunk rebuilding

[Byongho Lee]

This 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")'

Signed-off-by: Zach Brown <zab@redhat.com>
Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 chunk-recover.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 85dc1bca031a..b03330b4cda0 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -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;
 }
-- 
2.6.4

[PATCH v2 5/6] btrfs-progs: fix endian bug in update_super()

[Byongho Lee]

In 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().

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
 btrfs-image.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/btrfs-image.c b/btrfs-image.c
index 4a0a7c5414a7..bb1f63511647 100644
--- a/btrfs-image.c
+++ b/btrfs-image.c
@@ -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)
-- 
2.6.4

[PATCH v2 6/6] btrfs-progs: fix using on-disk structure to store in memory data

[Byongho Lee]

In '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.

Signed-off-by: Byongho Lee <bhlee.kernel@gmail.com>
---
v2:
  remove, "Signed-off-by: Zach Brown <zab@redhat.com>" 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(-)

diff --git a/qgroup-verify.c b/qgroup-verify.c
index 0ee52ff5c857..a0a4d4a5f5d9 100644
--- a/qgroup-verify.c
+++ b/qgroup-verify.c
@@ -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;
 
-- 
2.6.4

Re: [PATCH v2 0/6] btrfs-progs: fix endian bugs and clean-ups

[David Sterba]

On Mon, Jan 04, 2016 at 10:01:28AM +0900, Byongho Lee wrote:
> 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'.

>  - btrfs-progs: fix using on-disk structure to store in memory data:
>     remove, "Signed-off-by: Zach Brown <zab@redhat.com>" 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

All applied, thanks.