[v3,1/6] btrfs-progs: check: Export btrfs_type_to_imode
diff mbox series

Message ID 20190912031135.79696-2-wqu@suse.com
State New
Headers show
Series
  • btrfs-progs: check: Repair invalid inode mode in subvolume trees
Related show

Commit Message

Qu WenRuo Sept. 12, 2019, 3:11 a.m. UTC
This function will be later used by common mode code, so export it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        | 15 ---------------
 check/mode-common.h | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 15 deletions(-)

Comments

Nikolay Borisov Sept. 24, 2019, 8:41 a.m. UTC | #1
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

> +
> +	return imode_by_btrfs_type[(type)];
> +}
>  #endif
>
Qu WenRuo Sept. 24, 2019, 9:19 a.m. UTC | #2
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
>>

Patch
diff mbox series

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,
+	};
+
+	return imode_by_btrfs_type[(type)];
+}
 #endif