* [PATCH v3 1/2] btrfs-progs: export util functions about file extent items
@ 2021-07-14 14:54 Sidong Yang
2021-07-14 14:54 ` [PATCH v3 2/2] btrfs-progs: cmds: Add subcommand that dumps file extents Sidong Yang
2021-07-14 22:46 ` [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Qu Wenruo
0 siblings, 2 replies; 6+ messages in thread
From: Sidong Yang @ 2021-07-14 14:54 UTC (permalink / raw)
To: linux-btrfs, Qu Wenruo, Johannes Thumshirn; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] btrfs-progs: cmds: Add subcommand that dumps file extents
2021-07-14 14:54 [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Sidong Yang
@ 2021-07-14 14:54 ` Sidong Yang
2021-07-14 22:46 ` [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Qu Wenruo
1 sibling, 0 replies; 6+ messages in thread
From: Sidong Yang @ 2021-07-14 14:54 UTC (permalink / raw)
To: linux-btrfs, Qu Wenruo, Johannes Thumshirn; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items
2021-07-14 14:54 [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Sidong Yang
2021-07-14 14:54 ` [PATCH v3 2/2] btrfs-progs: cmds: Add subcommand that dumps file extents Sidong Yang
@ 2021-07-14 22:46 ` Qu Wenruo
2021-07-16 16:30 ` Sidong Yang
1 sibling, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2021-07-14 22:46 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs, Qu Wenruo, Johannes Thumshirn
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items
2021-07-14 22:46 ` [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Qu Wenruo
@ 2021-07-16 16:30 ` Sidong Yang
2021-07-16 22:56 ` Qu Wenruo
0 siblings, 1 reply; 6+ messages in thread
From: Sidong Yang @ 2021-07-16 16:30 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Qu Wenruo, Johannes Thumshirn
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
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items
2021-07-16 16:30 ` Sidong Yang
@ 2021-07-16 22:56 ` Qu Wenruo
2021-07-18 6:24 ` Sidong Yang
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2021-07-16 22:56 UTC (permalink / raw)
To: Sidong Yang, Qu Wenruo; +Cc: linux-btrfs, Johannes Thumshirn
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
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] btrfs-progs: export util functions about file extent items
2021-07-16 22:56 ` Qu Wenruo
@ 2021-07-18 6:24 ` Sidong Yang
0 siblings, 0 replies; 6+ messages in thread
From: Sidong Yang @ 2021-07-18 6:24 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, Johannes Thumshirn
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
> > > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-18 6:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14 14:54 [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Sidong Yang
2021-07-14 14:54 ` [PATCH v3 2/2] btrfs-progs: cmds: Add subcommand that dumps file extents Sidong Yang
2021-07-14 22:46 ` [PATCH v3 1/2] btrfs-progs: export util functions about file extent items Qu Wenruo
2021-07-16 16:30 ` Sidong Yang
2021-07-16 22:56 ` Qu Wenruo
2021-07-18 6:24 ` Sidong Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).