[2/4] btrfs-progs: Make btrfs-debug-tree print all readable strings for inode flags
diff mbox

Message ID 20161010031121.5017-3-quwenruo@cn.fujitsu.com
State Superseded
Headers show

Commit Message

Qu Wenruo Oct. 10, 2016, 3:11 a.m. UTC
Before this patch, only 3 inode flags have readable string: NODATACOW,
NODATASUM, READONLY.

This patch will output all readable strings for remaining inode flags,
making debug-tree tool more handy.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 print-tree.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

Comments

David Sterba Oct. 10, 2016, 3:50 p.m. UTC | #1
On Mon, Oct 10, 2016 at 11:11:19AM +0800, Qu Wenruo wrote:
> Before this patch, only 3 inode flags have readable string: NODATACOW,
> NODATASUM, READONLY.
> 
> This patch will output all readable strings for remaining inode flags,
> making debug-tree tool more handy.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  print-tree.c | 46 ++++++++++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/print-tree.c b/print-tree.c
> index 4444a14..f015646 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -851,29 +851,35 @@ static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
>  	}
>  }
>  
> -/* Caller should ensure sizeof(*ret) >= 29 "NODATASUM|NODATACOW|READONLY" */
> +#define copy_one_inode_flag(flags, name, empty, dst) ({			\
> +	if (flags & BTRFS_INODE_##name) {				\
> +		if (!empty)						\
> +			strcat(dst, "|");				\
> +		strcat(dst, #name);					\
> +		empty = 0;						\
> +	}								\
> +})

Can you please avoid using the macro? Or at least make it uppercase so
it's visible. Similar in the next patch.
--
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
Qu Wenruo Oct. 11, 2016, 2:18 a.m. UTC | #2
At 10/10/2016 11:50 PM, David Sterba wrote:
> On Mon, Oct 10, 2016 at 11:11:19AM +0800, Qu Wenruo wrote:
>> Before this patch, only 3 inode flags have readable string: NODATACOW,
>> NODATASUM, READONLY.
>>
>> This patch will output all readable strings for remaining inode flags,
>> making debug-tree tool more handy.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>  print-tree.c | 46 ++++++++++++++++++++++++++--------------------
>>  1 file changed, 26 insertions(+), 20 deletions(-)
>>
>> diff --git a/print-tree.c b/print-tree.c
>> index 4444a14..f015646 100644
>> --- a/print-tree.c
>> +++ b/print-tree.c
>> @@ -851,29 +851,35 @@ static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
>>  	}
>>  }
>>
>> -/* Caller should ensure sizeof(*ret) >= 29 "NODATASUM|NODATACOW|READONLY" */
>> +#define copy_one_inode_flag(flags, name, empty, dst) ({			\
>> +	if (flags & BTRFS_INODE_##name) {				\
>> +		if (!empty)						\
>> +			strcat(dst, "|");				\
>> +		strcat(dst, #name);					\
>> +		empty = 0;						\
>> +	}								\
>> +})
>
> Can you please avoid using the macro? Or at least make it uppercase so
> it's visible. Similar in the next patch.
>
>
OK, I'll change it to upper case.

The only reason I'm using macro is, inline function can't do 
stringification, or I missed something?

Thanks,
Qu


--
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
David Sterba Oct. 11, 2016, 10:07 a.m. UTC | #3
On Tue, Oct 11, 2016 at 10:18:51AM +0800, Qu Wenruo wrote:
> >> -/* Caller should ensure sizeof(*ret) >= 29 "NODATASUM|NODATACOW|READONLY" */
> >> +#define copy_one_inode_flag(flags, name, empty, dst) ({			\
> >> +	if (flags & BTRFS_INODE_##name) {				\
> >> +		if (!empty)						\
> >> +			strcat(dst, "|");				\
> >> +		strcat(dst, #name);					\
> >> +		empty = 0;						\
> >> +	}								\
> >> +})
> >
> > Can you please avoid using the macro? Or at least make it uppercase so
> > it's visible. Similar in the next patch.
> >
> >
> OK, I'll change it to upper case.

Ok.

> The only reason I'm using macro is, inline function can't do 
> stringification, or I missed something?

No, that's where macros help. My concern was about the hidden use of a
local variable, so at least an all-caps macro name would make it more
visible. As this is not going to be used elsewhere, we can live with
that.
--
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

Patch
diff mbox

diff --git a/print-tree.c b/print-tree.c
index 4444a14..f015646 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -851,29 +851,35 @@  static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
 	}
 }
 
-/* Caller should ensure sizeof(*ret) >= 29 "NODATASUM|NODATACOW|READONLY" */
+#define copy_one_inode_flag(flags, name, empty, dst) ({			\
+	if (flags & BTRFS_INODE_##name) {				\
+		if (!empty)						\
+			strcat(dst, "|");				\
+		strcat(dst, #name);					\
+		empty = 0;						\
+	}								\
+})
+
+/*
+ * Caller should ensure sizeof(*ret) >= 102: all charactors plus '|' of
+ * BTRFS_INODE_* flags
+ */
 static void inode_flags_to_str(u64 flags, char *ret)
 {
 	int empty = 1;
 
-	if (flags & BTRFS_INODE_NODATASUM) {
-		empty = 0;
-		strcpy(ret, "NODATASUM");
-	}
-	if (flags & BTRFS_INODE_NODATACOW) {
-		if (!empty) {
-			empty = 0;
-			strcat(ret, "|");
-		}
-		strcat(ret, "NODATACOW");
-	}
-	if (flags & BTRFS_INODE_READONLY) {
-		if (!empty) {
-			empty = 0;
-			strcat(ret, "|");
-		}
-		strcat(ret, "READONLY");
-	}
+	copy_one_inode_flag(flags, NODATASUM, empty, ret);
+	copy_one_inode_flag(flags, NODATACOW, empty, ret);
+	copy_one_inode_flag(flags, READONLY, empty, ret);
+	copy_one_inode_flag(flags, NOCOMPRESS, empty, ret);
+	copy_one_inode_flag(flags, PREALLOC, empty, ret);
+	copy_one_inode_flag(flags, SYNC, empty, ret);
+	copy_one_inode_flag(flags, IMMUTABLE, empty, ret);
+	copy_one_inode_flag(flags, APPEND, empty, ret);
+	copy_one_inode_flag(flags, NODUMP, empty, ret);
+	copy_one_inode_flag(flags, NOATIME, empty, ret);
+	copy_one_inode_flag(flags, DIRSYNC, empty, ret);
+	copy_one_inode_flag(flags, COMPRESS, empty, ret);
 	if (empty)
 		strcat(ret, "none");
 }
@@ -902,7 +908,7 @@  void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l)
 	u32 nr = btrfs_header_nritems(l);
 	u64 objectid;
 	u32 type;
-	char flags_str[32];
+	char flags_str[128];
 
 	printf("leaf %llu items %d free space %d generation %llu owner %llu\n",
 		(unsigned long long)btrfs_header_bytenr(l), nr,