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

[Qu Wenruo <wqu@suse.com>]

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
>