[PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[Sidong Yang]

This patch export two functions that convert enum about file extents to
string. It can be used in other code like inspect-internal command. And
this patch also make compress_type_to_str() function more safe by using
strncpy() than strcpy().

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
v2:
 - Prints type and compression
 - Use the terms from file_extents_item like disk_bytenr not like
   "physical"
v3:
 - export util functions for removing duplication
 - change the way to loop with search ioctl
---
 kernel-shared/print-tree.c | 18 ++++++++++--------
 kernel-shared/print-tree.h |  3 +++
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index e5d4b453..bfef7d26 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -27,6 +27,8 @@
 #include "kernel-shared/print-tree.h"
 #include "common/utils.h"
 
+#define COMPRESS_STR_LEN 5
+
 static void print_dir_item_type(struct extent_buffer *eb,
                                 struct btrfs_dir_item *di)
 {
@@ -338,27 +340,27 @@ static void print_uuids(struct extent_buffer *eb)
 	printf("fs uuid %s\nchunk uuid %s\n", fs_uuid, chunk_uuid);
 }
 
-static void compress_type_to_str(u8 compress_type, char *ret)
+void btrfs_compress_type_to_str(u8 compress_type, char *ret)
 {
 	switch (compress_type) {
 	case BTRFS_COMPRESS_NONE:
-		strcpy(ret, "none");
+		strncpy(ret, "none", COMPRESS_STR_LEN);
 		break;
 	case BTRFS_COMPRESS_ZLIB:
-		strcpy(ret, "zlib");
+		strncpy(ret, "zlib", COMPRESS_STR_LEN);
 		break;
 	case BTRFS_COMPRESS_LZO:
-		strcpy(ret, "lzo");
+		strncpy(ret, "lzo", COMPRESS_STR_LEN);
 		break;
 	case BTRFS_COMPRESS_ZSTD:
-		strcpy(ret, "zstd");
+		strncpy(ret, "zstd", COMPRESS_STR_LEN);
 		break;
 	default:
 		sprintf(ret, "UNKNOWN.%d", compress_type);
 	}
 }
 
-static const char* file_extent_type_to_str(u8 type)
+const char* btrfs_file_extent_type_to_str(u8 type)
 {
 	switch (type) {
 	case BTRFS_FILE_EXTENT_INLINE: return "inline";
@@ -376,12 +378,12 @@ static void print_file_extent_item(struct extent_buffer *eb,
 	unsigned char extent_type = btrfs_file_extent_type(eb, fi);
 	char compress_str[16];
 
-	compress_type_to_str(btrfs_file_extent_compression(eb, fi),
+	btrfs_compress_type_to_str(btrfs_file_extent_compression(eb, fi),
 			     compress_str);
 
 	printf("\t\tgeneration %llu type %hhu (%s)\n",
 			btrfs_file_extent_generation(eb, fi),
-			extent_type, file_extent_type_to_str(extent_type));
+			extent_type, btrfs_file_extent_type_to_str(extent_type));
 
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
 		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
diff --git a/kernel-shared/print-tree.h b/kernel-shared/print-tree.h
index 80fb6ef7..dbb2f183 100644
--- a/kernel-shared/print-tree.h
+++ b/kernel-shared/print-tree.h
@@ -43,4 +43,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type);
 void print_key_type(FILE *stream, u64 objectid, u8 type);
 void btrfs_print_superblock(struct btrfs_super_block *sb, int full);
 
+void btrfs_compress_type_to_str(u8 compress_type, char *ret);
+const char* btrfs_file_extent_type_to_str(u8 type);
+
 #endif
-- 
2.25.1

[PATCH v3 2/2] btrfs-progs: cmds: Add subcommand that dumps file extents

[Sidong Yang]

This patch adds an subcommand in inspect-internal. It dumps file extents of
the file that user provided. It helps to show the internal information
about file extents comprise the file.

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
v2:
 - Prints type and compression
 - Use the terms from file_extents_item like disk_bytenr not like
   "physical"
v3:
 - export util functions for removing duplication
 - change the way to loop with search ioctl
---
 Makefile                         |   2 +-
 cmds/commands.h                  |   2 +-
 cmds/inspect-dump-file-extents.c | 134 +++++++++++++++++++++++++++++++
 cmds/inspect.c                   |   1 +
 4 files changed, 137 insertions(+), 2 deletions(-)
 create mode 100644 cmds/inspect-dump-file-extents.c

diff --git a/Makefile b/Makefile
index a1cc457b..911e16de 100644
--- a/Makefile
+++ b/Makefile
@@ -156,7 +156,7 @@ cmds_objects = cmds/subvolume.o cmds/filesystem.o cmds/device.o cmds/scrub.o \
 	       cmds/restore.o cmds/rescue.o cmds/rescue-chunk-recover.o \
 	       cmds/rescue-super-recover.o \
 	       cmds/property.o cmds/filesystem-usage.o cmds/inspect-dump-tree.o \
-	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/filesystem-du.o \
+	       cmds/inspect-dump-super.o cmds/inspect-tree-stats.o cmds/inspect-dump-file-extents.o cmds/filesystem-du.o \
 	       mkfs/common.o check/mode-common.o check/mode-lowmem.o
 libbtrfs_objects = common/send-stream.o common/send-utils.o kernel-lib/rbtree.o btrfs-list.o \
 		   kernel-lib/radix-tree.o common/extent-cache.o kernel-shared/extent_io.o \
diff --git a/cmds/commands.h b/cmds/commands.h
index 8fa85d6c..55de248e 100644
--- a/cmds/commands.h
+++ b/cmds/commands.h
@@ -154,5 +154,5 @@ DECLARE_COMMAND(select_super);
 DECLARE_COMMAND(dump_super);
 DECLARE_COMMAND(debug_tree);
 DECLARE_COMMAND(rescue);
-
+DECLARE_COMMAND(inspect_dump_file_extents);
 #endif
diff --git a/cmds/inspect-dump-file-extents.c b/cmds/inspect-dump-file-extents.c
new file mode 100644
index 00000000..2497eeb1
--- /dev/null
+++ b/cmds/inspect-dump-file-extents.c
@@ -0,0 +1,134 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+#include <sys/ioctl.h>
+
+#include "common/utils.h"
+#include "cmds/commands.h"
+#include "kernel-shared/print-tree.h"
+
+static const char * const cmd_inspect_dump_file_extents_usage[] = {
+	"btrfs inspect-internal dump-extent path",
+	"Dump file extent in a textual form",
+	NULL
+};
+
+static int cmd_inspect_dump_file_extents(const struct cmd_struct *cmd,
+										 int argc, char **argv)
+{
+	int fd;
+	struct stat statbuf;
+	struct btrfs_ioctl_ino_lookup_args lookup;
+	struct btrfs_ioctl_search_args args;
+	struct btrfs_ioctl_search_key *sk = &args.key;
+	struct btrfs_file_extent_item *extent_item;
+	struct btrfs_ioctl_search_header *header;
+	u64 buf_off;
+	u64 len;
+	u64 start;
+	u64 disk_bytenr;
+	u64 disk_num_bytes;
+	u64 offset;
+	int ret;
+	int i;
+	char compress_str[16];
+
+	fd = open(argv[optind], O_RDONLY);
+	if (fd < 0) {
+		error("cannot open %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	if (fstat(fd, &statbuf) < 0) {
+		error("failed to fstat %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	lookup.treeid = 0;
+	lookup.objectid = BTRFS_FIRST_FREE_OBJECTID;
+
+	if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &lookup) < 0) {
+		error("failed to lookup inode %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	sk->tree_id = lookup.treeid;
+	sk->min_objectid = statbuf.st_ino;
+	sk->max_objectid = statbuf.st_ino;
+	sk->min_offset = 0;
+	sk->max_offset = UINT64_MAX;
+	sk->min_transid = 0;
+	sk->max_transid = UINT64_MAX;
+	sk->min_type = sk->max_type = BTRFS_EXTENT_DATA_KEY;
+	sk->nr_items = UINT32_MAX;
+
+again:
+	if (ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args)) {
+		error("failed to search tree ioctl %s: %m", argv[optind]);
+		ret = 1;
+		goto out;
+	}
+
+	buf_off = 0;
+	for(i=0; i<sk->nr_items; ++i) {
+		header = (struct btrfs_ioctl_search_header *)(args.buf + buf_off);
+
+		if (btrfs_search_header_type(header) == BTRFS_EXTENT_DATA_KEY) {
+			extent_item = (struct btrfs_file_extent_item *)(header + 1);
+			start = btrfs_search_header_offset(header);
+
+			printf("type = %s, start = %llu, ",
+				   btrfs_file_extent_type_to_str(extent_item->type), start);
+			switch (extent_item->type) {
+			case BTRFS_FILE_EXTENT_INLINE:
+				len = le64_to_cpu(extent_item->ram_bytes);
+				printf("len = %llu\n", len);
+				break;
+			case BTRFS_FILE_EXTENT_REG:
+			case BTRFS_FILE_EXTENT_PREALLOC:
+				len = le64_to_cpu(extent_item->num_bytes);
+				disk_bytenr = le64_to_cpu(extent_item->disk_bytenr);
+				disk_num_bytes = le64_to_cpu(extent_item->disk_num_bytes);
+				offset = le64_to_cpu(extent_item->offset);
+				btrfs_compress_type_to_str(extent_item->compression, compress_str);
+				printf("len = %llu, disk_bytenr = %llu, disk_num_bytes = %llu,"
+					   " offset = %llu, compression = %s\n",
+					   len, disk_bytenr, disk_num_bytes, offset, compress_str);
+
+				break;
+			}
+
+		}
+		buf_off += sizeof(*header) + btrfs_search_header_len(header);
+	}
+	if (sk->nr_items > 512) {
+		sk->nr_items = UINT32_MAX;
+		sk->min_offset = start + 1;
+		goto again;
+	}
+	ret = 0;
+out:
+	close(fd);
+	return ret;
+}
+DEFINE_SIMPLE_COMMAND(inspect_dump_file_extents, "dump-file-extents");
diff --git a/cmds/inspect.c b/cmds/inspect.c
index 2ef5f4b6..dfb0e27b 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -696,6 +696,7 @@ static const struct cmd_group inspect_cmd_group = {
 		&cmd_struct_inspect_dump_tree,
 		&cmd_struct_inspect_dump_super,
 		&cmd_struct_inspect_tree_stats,
+		&cmd_struct_inspect_dump_file_extents,
 		NULL
 	}
 };
-- 
2.25.1

Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[Qu Wenruo]

On 2021/7/14 下午10:54, Sidong Yang wrote:
> This patch export two functions that convert enum about file extents to
> string. It can be used in other code like inspect-internal command. And
> this patch also make compress_type_to_str() function more safe by using
> strncpy() than strcpy().
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> v2:
>   - Prints type and compression
>   - Use the terms from file_extents_item like disk_bytenr not like
>     "physical"
> v3:
>   - export util functions for removing duplication
>   - change the way to loop with search ioctl
> ---
>   kernel-shared/print-tree.c | 18 ++++++++++--------
>   kernel-shared/print-tree.h |  3 +++
>   2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
> index e5d4b453..bfef7d26 100644
> --- a/kernel-shared/print-tree.c
> +++ b/kernel-shared/print-tree.c
> @@ -27,6 +27,8 @@
>   #include "kernel-shared/print-tree.h"
>   #include "common/utils.h"
>   
> +#define COMPRESS_STR_LEN 5

It's already too small to contain all the strings.

We have a default branch:

  	default:
  		sprintf(ret, "UNKNOWN.%d", compress_type);

In that case, our minimal value should be strlen("UNKNOWN.255") + 1, 
which is 12 bytes.

Your old rounded up 16 bytes is in fact perfect in this case.

Despite that, this patch looks good to me.

Thanks,
Qu
> +
>   static void print_dir_item_type(struct extent_buffer *eb,
>                                   struct btrfs_dir_item *di)
>   {
> @@ -338,27 +340,27 @@ static void print_uuids(struct extent_buffer *eb)
>   	printf("fs uuid %s\nchunk uuid %s\n", fs_uuid, chunk_uuid);
>   }
>   
> -static void compress_type_to_str(u8 compress_type, char *ret)
> +void btrfs_compress_type_to_str(u8 compress_type, char *ret)
>   {
>   	switch (compress_type) {
>   	case BTRFS_COMPRESS_NONE:
> -		strcpy(ret, "none");
> +		strncpy(ret, "none", COMPRESS_STR_LEN);
>   		break;
>   	case BTRFS_COMPRESS_ZLIB:
> -		strcpy(ret, "zlib");
> +		strncpy(ret, "zlib", COMPRESS_STR_LEN);
>   		break;
>   	case BTRFS_COMPRESS_LZO:
> -		strcpy(ret, "lzo");
> +		strncpy(ret, "lzo", COMPRESS_STR_LEN);
>   		break;
>   	case BTRFS_COMPRESS_ZSTD:
> -		strcpy(ret, "zstd");
> +		strncpy(ret, "zstd", COMPRESS_STR_LEN);
>   		break;
>   	default:
>   		sprintf(ret, "UNKNOWN.%d", compress_type);
>   	}
>   }
>   
> -static const char* file_extent_type_to_str(u8 type)
> +const char* btrfs_file_extent_type_to_str(u8 type)
>   {
>   	switch (type) {
>   	case BTRFS_FILE_EXTENT_INLINE: return "inline";
> @@ -376,12 +378,12 @@ static void print_file_extent_item(struct extent_buffer *eb,
>   	unsigned char extent_type = btrfs_file_extent_type(eb, fi);
>   	char compress_str[16];
>   
> -	compress_type_to_str(btrfs_file_extent_compression(eb, fi),
> +	btrfs_compress_type_to_str(btrfs_file_extent_compression(eb, fi),
>   			     compress_str);
>   
>   	printf("\t\tgeneration %llu type %hhu (%s)\n",
>   			btrfs_file_extent_generation(eb, fi),
> -			extent_type, file_extent_type_to_str(extent_type));
> +			extent_type, btrfs_file_extent_type_to_str(extent_type));
>   
>   	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>   		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
> diff --git a/kernel-shared/print-tree.h b/kernel-shared/print-tree.h
> index 80fb6ef7..dbb2f183 100644
> --- a/kernel-shared/print-tree.h
> +++ b/kernel-shared/print-tree.h
> @@ -43,4 +43,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type);
>   void print_key_type(FILE *stream, u64 objectid, u8 type);
>   void btrfs_print_superblock(struct btrfs_super_block *sb, int full);
>   
> +void btrfs_compress_type_to_str(u8 compress_type, char *ret);
> +const char* btrfs_file_extent_type_to_str(u8 type);
> +
>   #endif
> 

Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[Sidong Yang]

On Thu, Jul 15, 2021 at 06:46:08AM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/7/14 下午10:54, Sidong Yang wrote:
> > This patch export two functions that convert enum about file extents to
> > string. It can be used in other code like inspect-internal command. And
> > this patch also make compress_type_to_str() function more safe by using
> > strncpy() than strcpy().
> > 
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> > v2:
> >   - Prints type and compression
> >   - Use the terms from file_extents_item like disk_bytenr not like
> >     "physical"
> > v3:
> >   - export util functions for removing duplication
> >   - change the way to loop with search ioctl
> > ---
> >   kernel-shared/print-tree.c | 18 ++++++++++--------
> >   kernel-shared/print-tree.h |  3 +++
> >   2 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
> > index e5d4b453..bfef7d26 100644
> > --- a/kernel-shared/print-tree.c
> > +++ b/kernel-shared/print-tree.c
> > @@ -27,6 +27,8 @@
> >   #include "kernel-shared/print-tree.h"
> >   #include "common/utils.h"
> > +#define COMPRESS_STR_LEN 5
> 
> It's already too small to contain all the strings.
> 
> We have a default branch:
> 
>  	default:
>  		sprintf(ret, "UNKNOWN.%d", compress_type);
> 
> In that case, our minimal value should be strlen("UNKNOWN.255") + 1, which
> is 12 bytes.
> 
> Your old rounded up 16 bytes is in fact perfect in this case.
> 
> Despite that, this patch looks good to me.

Thanks for review.

I've been thought about this. should COMPRESS_STR_LEN be exported?
Developer that use this function needs to know max length of string.
Or we can choose the simple implementation that just return const char*
like btrfs_file_extent_type_to_str(). But it will be hard to print
unknown compress_type.

Thanks,
Sidong
> 
> Thanks,
> Qu
> > +
> >   static void print_dir_item_type(struct extent_buffer *eb,
> >                                   struct btrfs_dir_item *di)
> >   {
> > @@ -338,27 +340,27 @@ static void print_uuids(struct extent_buffer *eb)
> >   	printf("fs uuid %s\nchunk uuid %s\n", fs_uuid, chunk_uuid);
> >   }
> > -static void compress_type_to_str(u8 compress_type, char *ret)
> > +void btrfs_compress_type_to_str(u8 compress_type, char *ret)
> >   {
> >   	switch (compress_type) {
> >   	case BTRFS_COMPRESS_NONE:
> > -		strcpy(ret, "none");
> > +		strncpy(ret, "none", COMPRESS_STR_LEN);
> >   		break;
> >   	case BTRFS_COMPRESS_ZLIB:
> > -		strcpy(ret, "zlib");
> > +		strncpy(ret, "zlib", COMPRESS_STR_LEN);
> >   		break;
> >   	case BTRFS_COMPRESS_LZO:
> > -		strcpy(ret, "lzo");
> > +		strncpy(ret, "lzo", COMPRESS_STR_LEN);
> >   		break;
> >   	case BTRFS_COMPRESS_ZSTD:
> > -		strcpy(ret, "zstd");
> > +		strncpy(ret, "zstd", COMPRESS_STR_LEN);
> >   		break;
> >   	default:
> >   		sprintf(ret, "UNKNOWN.%d", compress_type);
> >   	}
> >   }
> > -static const char* file_extent_type_to_str(u8 type)
> > +const char* btrfs_file_extent_type_to_str(u8 type)
> >   {
> >   	switch (type) {
> >   	case BTRFS_FILE_EXTENT_INLINE: return "inline";
> > @@ -376,12 +378,12 @@ static void print_file_extent_item(struct extent_buffer *eb,
> >   	unsigned char extent_type = btrfs_file_extent_type(eb, fi);
> >   	char compress_str[16];
> > -	compress_type_to_str(btrfs_file_extent_compression(eb, fi),
> > +	btrfs_compress_type_to_str(btrfs_file_extent_compression(eb, fi),
> >   			     compress_str);
> >   	printf("\t\tgeneration %llu type %hhu (%s)\n",
> >   			btrfs_file_extent_generation(eb, fi),
> > -			extent_type, file_extent_type_to_str(extent_type));
> > +			extent_type, btrfs_file_extent_type_to_str(extent_type));
> >   	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> >   		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
> > diff --git a/kernel-shared/print-tree.h b/kernel-shared/print-tree.h
> > index 80fb6ef7..dbb2f183 100644
> > --- a/kernel-shared/print-tree.h
> > +++ b/kernel-shared/print-tree.h
> > @@ -43,4 +43,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type);
> >   void print_key_type(FILE *stream, u64 objectid, u8 type);
> >   void btrfs_print_superblock(struct btrfs_super_block *sb, int full);
> > +void btrfs_compress_type_to_str(u8 compress_type, char *ret);
> > +const char* btrfs_file_extent_type_to_str(u8 type);
> > +
> >   #endif
> > 
> 

Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[Qu Wenruo]

On 2021/7/17 上午12:30, Sidong Yang wrote:
> On Thu, Jul 15, 2021 at 06:46:08AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/7/14 下午10:54, Sidong Yang wrote:
>>> This patch export two functions that convert enum about file extents to
>>> string. It can be used in other code like inspect-internal command. And
>>> this patch also make compress_type_to_str() function more safe by using
>>> strncpy() than strcpy().
>>>
>>> Signed-off-by: Sidong Yang <realwakka@gmail.com>
>>> ---
>>> v2:
>>>    - Prints type and compression
>>>    - Use the terms from file_extents_item like disk_bytenr not like
>>>      "physical"
>>> v3:
>>>    - export util functions for removing duplication
>>>    - change the way to loop with search ioctl
>>> ---
>>>    kernel-shared/print-tree.c | 18 ++++++++++--------
>>>    kernel-shared/print-tree.h |  3 +++
>>>    2 files changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
>>> index e5d4b453..bfef7d26 100644
>>> --- a/kernel-shared/print-tree.c
>>> +++ b/kernel-shared/print-tree.c
>>> @@ -27,6 +27,8 @@
>>>    #include "kernel-shared/print-tree.h"
>>>    #include "common/utils.h"
>>> +#define COMPRESS_STR_LEN 5
>>
>> It's already too small to contain all the strings.
>>
>> We have a default branch:
>>
>>   	default:
>>   		sprintf(ret, "UNKNOWN.%d", compress_type);
>>
>> In that case, our minimal value should be strlen("UNKNOWN.255") + 1, which
>> is 12 bytes.
>>
>> Your old rounded up 16 bytes is in fact perfect in this case.
>>
>> Despite that, this patch looks good to me.
>
> Thanks for review.
>
> I've been thought about this. should COMPRESS_STR_LEN be exported?

I guess yes.

Thanks,
Qu

> Developer that use this function needs to know max length of string.
> Or we can choose the simple implementation that just return const char*
> like btrfs_file_extent_type_to_str(). But it will be hard to print
> unknown compress_type.
>
> Thanks,
> Sidong
>>
>> Thanks,
>> Qu
>>> +
>>>    static void print_dir_item_type(struct extent_buffer *eb,
>>>                                    struct btrfs_dir_item *di)
>>>    {
>>> @@ -338,27 +340,27 @@ static void print_uuids(struct extent_buffer *eb)
>>>    	printf("fs uuid %s\nchunk uuid %s\n", fs_uuid, chunk_uuid);
>>>    }
>>> -static void compress_type_to_str(u8 compress_type, char *ret)
>>> +void btrfs_compress_type_to_str(u8 compress_type, char *ret)
>>>    {
>>>    	switch (compress_type) {
>>>    	case BTRFS_COMPRESS_NONE:
>>> -		strcpy(ret, "none");
>>> +		strncpy(ret, "none", COMPRESS_STR_LEN);
>>>    		break;
>>>    	case BTRFS_COMPRESS_ZLIB:
>>> -		strcpy(ret, "zlib");
>>> +		strncpy(ret, "zlib", COMPRESS_STR_LEN);
>>>    		break;
>>>    	case BTRFS_COMPRESS_LZO:
>>> -		strcpy(ret, "lzo");
>>> +		strncpy(ret, "lzo", COMPRESS_STR_LEN);
>>>    		break;
>>>    	case BTRFS_COMPRESS_ZSTD:
>>> -		strcpy(ret, "zstd");
>>> +		strncpy(ret, "zstd", COMPRESS_STR_LEN);
>>>    		break;
>>>    	default:
>>>    		sprintf(ret, "UNKNOWN.%d", compress_type);
>>>    	}
>>>    }
>>> -static const char* file_extent_type_to_str(u8 type)
>>> +const char* btrfs_file_extent_type_to_str(u8 type)
>>>    {
>>>    	switch (type) {
>>>    	case BTRFS_FILE_EXTENT_INLINE: return "inline";
>>> @@ -376,12 +378,12 @@ static void print_file_extent_item(struct extent_buffer *eb,
>>>    	unsigned char extent_type = btrfs_file_extent_type(eb, fi);
>>>    	char compress_str[16];
>>> -	compress_type_to_str(btrfs_file_extent_compression(eb, fi),
>>> +	btrfs_compress_type_to_str(btrfs_file_extent_compression(eb, fi),
>>>    			     compress_str);
>>>    	printf("\t\tgeneration %llu type %hhu (%s)\n",
>>>    			btrfs_file_extent_generation(eb, fi),
>>> -			extent_type, file_extent_type_to_str(extent_type));
>>> +			extent_type, btrfs_file_extent_type_to_str(extent_type));
>>>    	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>>>    		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
>>> diff --git a/kernel-shared/print-tree.h b/kernel-shared/print-tree.h
>>> index 80fb6ef7..dbb2f183 100644
>>> --- a/kernel-shared/print-tree.h
>>> +++ b/kernel-shared/print-tree.h
>>> @@ -43,4 +43,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type);
>>>    void print_key_type(FILE *stream, u64 objectid, u8 type);
>>>    void btrfs_print_superblock(struct btrfs_super_block *sb, int full);
>>> +void btrfs_compress_type_to_str(u8 compress_type, char *ret);
>>> +const char* btrfs_file_extent_type_to_str(u8 type);
>>> +
>>>    #endif
>>>
>>

Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items

[Sidong Yang]

On Sat, Jul 17, 2021 at 06:56:37AM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/7/17 上午12:30, Sidong Yang wrote:
> > On Thu, Jul 15, 2021 at 06:46:08AM +0800, Qu Wenruo wrote:
> > > 
> > > 
> > > On 2021/7/14 下午10:54, Sidong Yang wrote:
> > > > This patch export two functions that convert enum about file extents to
> > > > string. It can be used in other code like inspect-internal command. And
> > > > this patch also make compress_type_to_str() function more safe by using
> > > > strncpy() than strcpy().
> > > > 
> > > > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > > > ---
> > > > v2:
> > > >    - Prints type and compression
> > > >    - Use the terms from file_extents_item like disk_bytenr not like
> > > >      "physical"
> > > > v3:
> > > >    - export util functions for removing duplication
> > > >    - change the way to loop with search ioctl
> > > > ---
> > > >    kernel-shared/print-tree.c | 18 ++++++++++--------
> > > >    kernel-shared/print-tree.h |  3 +++
> > > >    2 files changed, 13 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
> > > > index e5d4b453..bfef7d26 100644
> > > > --- a/kernel-shared/print-tree.c
> > > > +++ b/kernel-shared/print-tree.c
> > > > @@ -27,6 +27,8 @@
> > > >    #include "kernel-shared/print-tree.h"
> > > >    #include "common/utils.h"
> > > > +#define COMPRESS_STR_LEN 5
> > > 
> > > It's already too small to contain all the strings.
> > > 
> > > We have a default branch:
> > > 
> > >   	default:
> > >   		sprintf(ret, "UNKNOWN.%d", compress_type);
> > > 
> > > In that case, our minimal value should be strlen("UNKNOWN.255") + 1, which
> > > is 12 bytes.
> > > 
> > > Your old rounded up 16 bytes is in fact perfect in this case.
> > > 
> > > Despite that, this patch looks good to me.
> > 
> > Thanks for review.
> > 
> > I've been thought about this. should COMPRESS_STR_LEN be exported?
> 
> I guess yes.

Thanks, I'll apply this.

Thanks,
Sidong
> 
> Thanks,
> Qu
> 
> > Developer that use this function needs to know max length of string.
> > Or we can choose the simple implementation that just return const char*
> > like btrfs_file_extent_type_to_str(). But it will be hard to print
> > unknown compress_type.
> > 
> > Thanks,
> > Sidong
> > > 
> > > Thanks,
> > > Qu
> > > > +
> > > >    static void print_dir_item_type(struct extent_buffer *eb,
> > > >                                    struct btrfs_dir_item *di)
> > > >    {
> > > > @@ -338,27 +340,27 @@ static void print_uuids(struct extent_buffer *eb)
> > > >    	printf("fs uuid %s\nchunk uuid %s\n", fs_uuid, chunk_uuid);
> > > >    }
> > > > -static void compress_type_to_str(u8 compress_type, char *ret)
> > > > +void btrfs_compress_type_to_str(u8 compress_type, char *ret)
> > > >    {
> > > >    	switch (compress_type) {
> > > >    	case BTRFS_COMPRESS_NONE:
> > > > -		strcpy(ret, "none");
> > > > +		strncpy(ret, "none", COMPRESS_STR_LEN);
> > > >    		break;
> > > >    	case BTRFS_COMPRESS_ZLIB:
> > > > -		strcpy(ret, "zlib");
> > > > +		strncpy(ret, "zlib", COMPRESS_STR_LEN);
> > > >    		break;
> > > >    	case BTRFS_COMPRESS_LZO:
> > > > -		strcpy(ret, "lzo");
> > > > +		strncpy(ret, "lzo", COMPRESS_STR_LEN);
> > > >    		break;
> > > >    	case BTRFS_COMPRESS_ZSTD:
> > > > -		strcpy(ret, "zstd");
> > > > +		strncpy(ret, "zstd", COMPRESS_STR_LEN);
> > > >    		break;
> > > >    	default:
> > > >    		sprintf(ret, "UNKNOWN.%d", compress_type);
> > > >    	}
> > > >    }
> > > > -static const char* file_extent_type_to_str(u8 type)
> > > > +const char* btrfs_file_extent_type_to_str(u8 type)
> > > >    {
> > > >    	switch (type) {
> > > >    	case BTRFS_FILE_EXTENT_INLINE: return "inline";
> > > > @@ -376,12 +378,12 @@ static void print_file_extent_item(struct extent_buffer *eb,
> > > >    	unsigned char extent_type = btrfs_file_extent_type(eb, fi);
> > > >    	char compress_str[16];
> > > > -	compress_type_to_str(btrfs_file_extent_compression(eb, fi),
> > > > +	btrfs_compress_type_to_str(btrfs_file_extent_compression(eb, fi),
> > > >    			     compress_str);
> > > >    	printf("\t\tgeneration %llu type %hhu (%s)\n",
> > > >    			btrfs_file_extent_generation(eb, fi),
> > > > -			extent_type, file_extent_type_to_str(extent_type));
> > > > +			extent_type, btrfs_file_extent_type_to_str(extent_type));
> > > >    	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> > > >    		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
> > > > diff --git a/kernel-shared/print-tree.h b/kernel-shared/print-tree.h
> > > > index 80fb6ef7..dbb2f183 100644
> > > > --- a/kernel-shared/print-tree.h
> > > > +++ b/kernel-shared/print-tree.h
> > > > @@ -43,4 +43,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type);
> > > >    void print_key_type(FILE *stream, u64 objectid, u8 type);
> > > >    void btrfs_print_superblock(struct btrfs_super_block *sb, int full);
> > > > +void btrfs_compress_type_to_str(u8 compress_type, char *ret);
> > > > +const char* btrfs_file_extent_type_to_str(u8 type);
> > > > +
> > > >    #endif
> > > > 
> > >