Re: [PATCH v3 1/6] btrfs-progs: check: Export btrfs_type_to_imode

[Qu WenRuo <wqu@suse.com>]

On 2019/9/24 下午4:41, Nikolay Borisov wrote:
> 
> 
> On 12.09.19 г. 6:11 ч., Qu Wenruo wrote:
>> This function will be later used by common mode code, so export it.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com> but see one nit below.
> 
>> ---
>>  check/main.c        | 15 ---------------
>>  check/mode-common.h | 15 +++++++++++++++
>>  2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 2e16b4e6f05b..902279740589 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -2448,21 +2448,6 @@ out:
>>  	return ret;
>>  }
>>  
>> -static u32 btrfs_type_to_imode(u8 type)
>> -{
>> -	static u32 imode_by_btrfs_type[] = {
>> -		[BTRFS_FT_REG_FILE]	= S_IFREG,
>> -		[BTRFS_FT_DIR]		= S_IFDIR,
>> -		[BTRFS_FT_CHRDEV]	= S_IFCHR,
>> -		[BTRFS_FT_BLKDEV]	= S_IFBLK,
>> -		[BTRFS_FT_FIFO]		= S_IFIFO,
>> -		[BTRFS_FT_SOCK]		= S_IFSOCK,
>> -		[BTRFS_FT_SYMLINK]	= S_IFLNK,
>> -	};
>> -
>> -	return imode_by_btrfs_type[(type)];
>> -}
>> -
>>  static int repair_inode_no_item(struct btrfs_trans_handle *trans,
>>  				struct btrfs_root *root,
>>  				struct btrfs_path *path,
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 161b84a8deb0..6c8d6d7578a6 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -156,4 +156,19 @@ static inline bool is_valid_imode(u32 imode)
>>  }
>>  
>>  int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
>> +
>> +static inline u32 btrfs_type_to_imode(u8 type)
>> +{
>> +	static u32 imode_by_btrfs_type[] = {
>> +		[BTRFS_FT_REG_FILE]	= S_IFREG,
>> +		[BTRFS_FT_DIR]		= S_IFDIR,
>> +		[BTRFS_FT_CHRDEV]	= S_IFCHR,
>> +		[BTRFS_FT_BLKDEV]	= S_IFBLK,
>> +		[BTRFS_FT_FIFO]		= S_IFIFO,
>> +		[BTRFS_FT_SOCK]		= S_IFSOCK,
>> +		[BTRFS_FT_SYMLINK]	= S_IFLNK,
>> +	};
> 
> nit: If the array is defined in a function in a header this means it
> will be copied to every object file this header is included so it will
> result in a minor bloat of size. It might better to have it defined in
> check/main.c and have it declared extern in mode-common.h

I'm wondering what happens in the final binary.

IIRC there should be only one copy of the static const array in .data
segment.

So that should be mostly OK I guess?

Thanks,
Qu

> 
>> +
>> +	return imode_by_btrfs_type[(type)];
>> +}
>>  #endif
>>