diff mbox

[v2] btrfs-progs: mkfs: Replace number with a macro

Message ID 20170626101829.16021-1-gujx@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gu Jinxiang June 26, 2017, 10:18 a.m. UTC
For code maintainability and scalability,
replace number with a macro of member blocks in btrfs_mkfs_config.

Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
---
Changes since v1:
Missing a using place. And modify it.

 mkfs/common.c | 4 ++--
 mkfs/common.h | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

David Sterba June 26, 2017, 2:27 p.m. UTC | #1
On Mon, Jun 26, 2017 at 06:18:29PM +0800, Gu Jinxiang wrote:
> For code maintainability and scalability,
> replace number with a macro of member blocks in btrfs_mkfs_config.
> 
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
> ---
> Changes since v1:
> Missing a using place. And modify it.
> 
>  mkfs/common.c | 4 ++--
>  mkfs/common.h | 5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index e4785c5..0d79650 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -94,7 +94,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  	uuid_generate(chunk_tree_uuid);
>  
>  	cfg->blocks[0] = BTRFS_SUPER_INFO_OFFSET;
> -	for (i = 1; i < 7; i++) {
> +	for (i = 1; i <= BTRFS_MKFS_ROOTS_NR; i++) {

I'm not sure this is the best way to make the code more readable. "NR"
is the count of the roots and if it were used as " < NR" then it's clear
that we're iterating over a given number of items, but here the count is
also going to be used as an index to an array.

While this is correct, it's still necessary to keep in mind that some +1
or <= is needed while dealing with the blocks.

make_btrfs could use some heavy cleanup so we don't rely on the
hardcoded constants, in a similar way to reference_root_table so we can
use symbolic tree names.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/mkfs/common.c b/mkfs/common.c
index e4785c5..0d79650 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -94,7 +94,7 @@  int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	uuid_generate(chunk_tree_uuid);
 
 	cfg->blocks[0] = BTRFS_SUPER_INFO_OFFSET;
-	for (i = 1; i < 7; i++) {
+	for (i = 1; i <= BTRFS_MKFS_ROOTS_NR; i++) {
 		cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + 1024 * 1024 +
 			cfg->nodesize * i;
 	}
@@ -210,7 +210,7 @@  int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		cfg->nodesize - sizeof(struct btrfs_header));
 	nritems = 0;
 	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
-	for (i = 1; i < 7; i++) {
+	for (i = 1; i <= BTRFS_MKFS_ROOTS_NR; i++) {
 		item_size = sizeof(struct btrfs_extent_item);
 		if (!skinny_metadata)
 			item_size += sizeof(struct btrfs_tree_block_info);
diff --git a/mkfs/common.h b/mkfs/common.h
index 666a75b..e23e79b 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -28,6 +28,9 @@ 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE SZ_4M
 #define BTRFS_MKFS_SMALL_VOLUME_SIZE SZ_1G
 
+/* roots: root tree, extent tree, chunk tree, dev tree, fs tree, csum tree */
+#define BTRFS_MKFS_ROOTS_NR 6
+
 struct btrfs_mkfs_config {
 	/* Label of the new filesystem */
 	const char *label;
@@ -43,7 +46,7 @@  struct btrfs_mkfs_config {
 	/* Output fields, set during creation */
 
 	/* Logical addresses of superblock [0] and other tree roots */
-	u64 blocks[8];
+	u64 blocks[BTRFS_MKFS_ROOTS_NR + 1];
 	char fs_uuid[BTRFS_UUID_UNPARSED_SIZE];
 	char chunk_uuid[BTRFS_UUID_UNPARSED_SIZE];