| Message ID | 20190912031135.79696-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: check: Repair invalid inode mode in subvolume trees | expand |
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 >
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 >>
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
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(-)