* [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement
@ 2017-01-11 6:30 Amir Goldstein
2017-01-11 16:23 ` Eric Sandeen
0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2017-01-11 6:30 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Darrick J . Wong, Brian Foster, Christoph Hellwig, linux-xfs
The size of the xfs_mode_to_ftype[] conversion table
was too small to handle an invalid value of mode=S_IFMT.
Instead of fixing the table size, replace the conversion table
with a conversion helper that uses a switch statement.
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
libxfs/xfs_dir2.c | 36 ++++++++++++++++++++++--------------
libxfs/xfs_dir2.h | 5 ++---
repair/dino_chunks.c | 6 +-----
repair/phase6.c | 2 +-
4 files changed, 26 insertions(+), 23 deletions(-)
Eric,
This is a matching patch to the one that got Reviewed-by Darrick for kernel.
Tested xfs_repair with the new xfs/348 test with -n ftype=0|1.
Amir.
v3:
- Replaced xfs_mode_to_ftype table with switch statement
v2:
- remove wrong argument about on-disk malformed mode from commit message
- address Brian's review comments
- test with new xfs/348
v1:
- fix patch for xfsprogs derived from kernel patch
diff --git a/libxfs/xfs_dir2.c b/libxfs/xfs_dir2.c
index 4180a93..257a99b 100644
--- a/libxfs/xfs_dir2.c
+++ b/libxfs/xfs_dir2.c
@@ -34,21 +34,29 @@
struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
/*
- * @mode, if set, indicates that the type field needs to be set up.
- * This uses the transformation from file mode to DT_* as defined in linux/fs.h
- * for file type specification. This will be propagated into the directory
- * structure if appropriate for the given operation and filesystem config.
+ * Convert inode mode to directory entry filetype
*/
-const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
- [0] = XFS_DIR3_FT_UNKNOWN,
- [S_IFREG >> S_SHIFT] = XFS_DIR3_FT_REG_FILE,
- [S_IFDIR >> S_SHIFT] = XFS_DIR3_FT_DIR,
- [S_IFCHR >> S_SHIFT] = XFS_DIR3_FT_CHRDEV,
- [S_IFBLK >> S_SHIFT] = XFS_DIR3_FT_BLKDEV,
- [S_IFIFO >> S_SHIFT] = XFS_DIR3_FT_FIFO,
- [S_IFSOCK >> S_SHIFT] = XFS_DIR3_FT_SOCK,
- [S_IFLNK >> S_SHIFT] = XFS_DIR3_FT_SYMLINK,
-};
+const unsigned char xfs_mode_to_ftype(int mode)
+{
+ switch (mode & S_IFMT) {
+ case S_IFREG:
+ return XFS_DIR3_FT_REG_FILE;
+ case S_IFDIR:
+ return XFS_DIR3_FT_DIR;
+ case S_IFCHR:
+ return XFS_DIR3_FT_CHRDEV;
+ case S_IFBLK:
+ return XFS_DIR3_FT_BLKDEV;
+ case S_IFIFO:
+ return XFS_DIR3_FT_FIFO;
+ case S_IFSOCK:
+ return XFS_DIR3_FT_SOCK;
+ case S_IFLNK:
+ return XFS_DIR3_FT_SYMLINK;
+ default:
+ return XFS_DIR3_FT_UNKNOWN;
+ }
+}
/*
* ASCII case-insensitive (ie. A-Z) support for directories that was
diff --git a/libxfs/xfs_dir2.h b/libxfs/xfs_dir2.h
index 89b9e24..ea20c12 100644
--- a/libxfs/xfs_dir2.h
+++ b/libxfs/xfs_dir2.h
@@ -32,10 +32,9 @@ struct xfs_dir2_data_unused;
extern struct xfs_name xfs_name_dotdot;
/*
- * directory filetype conversion tables.
+ * Convert inode mode to directory entry filetype
*/
-#define S_SHIFT 12
-extern const unsigned char xfs_mode_to_ftype[];
+extern const unsigned char xfs_mode_to_ftype(int mode);
/*
* directory operations vector for encode/decode routines
diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 4db9512..a3909ac 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -826,8 +826,6 @@ next_readbuf:
* we do now, this is where to start.
*/
if (is_used) {
- __uint16_t di_mode;
-
if (is_inode_free(ino_rec, irec_offset)) {
if (verbose || no_modify) {
do_warn(
@@ -846,10 +844,8 @@ next_readbuf:
* store the on-disk file type for comparing in
* phase 6.
*/
- di_mode = be16_to_cpu(dino->di_mode);
- di_mode = (di_mode & S_IFMT) >> S_SHIFT;
set_inode_ftype(ino_rec, irec_offset,
- xfs_mode_to_ftype[di_mode]);
+ xfs_mode_to_ftype(be16_to_cpu(dino->di_mode)));
/*
* store on-disk nlink count for comparing in phase 7
diff --git a/repair/phase6.c b/repair/phase6.c
index 06eed16..c492dd9 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1082,7 +1082,7 @@ mv_orphanage(
if ((err = -libxfs_iget(mp, NULL, ino, 0, &ino_p)))
do_error(_("%d - couldn't iget disconnected inode\n"), err);
- xname.type = xfs_mode_to_ftype[(VFS_I(ino_p)->i_mode & S_IFMT)>>S_SHIFT];
+ xname.type = xfs_mode_to_ftype(VFS_I(ino_p)->i_mode);
if (isa_dir) {
irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, orphanage_ino),
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement
2017-01-11 6:30 [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement Amir Goldstein
@ 2017-01-11 16:23 ` Eric Sandeen
2017-01-20 10:58 ` Amir Goldstein
0 siblings, 1 reply; 4+ messages in thread
From: Eric Sandeen @ 2017-01-11 16:23 UTC (permalink / raw)
To: Amir Goldstein
Cc: Darrick J . Wong, Brian Foster, Christoph Hellwig, linux-xfs
On 1/11/17 12:30 AM, Amir Goldstein wrote:
> The size of the xfs_mode_to_ftype[] conversion table
> was too small to handle an invalid value of mode=S_IFMT.
>
> Instead of fixing the table size, replace the conversion table
> with a conversion helper that uses a switch statement.
>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> libxfs/xfs_dir2.c | 36 ++++++++++++++++++++++--------------
> libxfs/xfs_dir2.h | 5 ++---
> repair/dino_chunks.c | 6 +-----
> repair/phase6.c | 2 +-
> 4 files changed, 26 insertions(+), 23 deletions(-)
>
> Eric,
>
> This is a matching patch to the one that got Reviewed-by Darrick for kernel.
>
> Tested xfs_repair with the new xfs/348 test with -n ftype=0|1.
Thanks, I'll pull this in after Darrick commits it to the kernel.
-Eric
> Amir.
>
> v3:
> - Replaced xfs_mode_to_ftype table with switch statement
>
> v2:
> - remove wrong argument about on-disk malformed mode from commit message
> - address Brian's review comments
> - test with new xfs/348
>
> v1:
> - fix patch for xfsprogs derived from kernel patch
>
> diff --git a/libxfs/xfs_dir2.c b/libxfs/xfs_dir2.c
> index 4180a93..257a99b 100644
> --- a/libxfs/xfs_dir2.c
> +++ b/libxfs/xfs_dir2.c
> @@ -34,21 +34,29 @@
> struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
>
> /*
> - * @mode, if set, indicates that the type field needs to be set up.
> - * This uses the transformation from file mode to DT_* as defined in linux/fs.h
> - * for file type specification. This will be propagated into the directory
> - * structure if appropriate for the given operation and filesystem config.
> + * Convert inode mode to directory entry filetype
> */
> -const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] = {
> - [0] = XFS_DIR3_FT_UNKNOWN,
> - [S_IFREG >> S_SHIFT] = XFS_DIR3_FT_REG_FILE,
> - [S_IFDIR >> S_SHIFT] = XFS_DIR3_FT_DIR,
> - [S_IFCHR >> S_SHIFT] = XFS_DIR3_FT_CHRDEV,
> - [S_IFBLK >> S_SHIFT] = XFS_DIR3_FT_BLKDEV,
> - [S_IFIFO >> S_SHIFT] = XFS_DIR3_FT_FIFO,
> - [S_IFSOCK >> S_SHIFT] = XFS_DIR3_FT_SOCK,
> - [S_IFLNK >> S_SHIFT] = XFS_DIR3_FT_SYMLINK,
> -};
> +const unsigned char xfs_mode_to_ftype(int mode)
> +{
> + switch (mode & S_IFMT) {
> + case S_IFREG:
> + return XFS_DIR3_FT_REG_FILE;
> + case S_IFDIR:
> + return XFS_DIR3_FT_DIR;
> + case S_IFCHR:
> + return XFS_DIR3_FT_CHRDEV;
> + case S_IFBLK:
> + return XFS_DIR3_FT_BLKDEV;
> + case S_IFIFO:
> + return XFS_DIR3_FT_FIFO;
> + case S_IFSOCK:
> + return XFS_DIR3_FT_SOCK;
> + case S_IFLNK:
> + return XFS_DIR3_FT_SYMLINK;
> + default:
> + return XFS_DIR3_FT_UNKNOWN;
> + }
> +}
>
> /*
> * ASCII case-insensitive (ie. A-Z) support for directories that was
> diff --git a/libxfs/xfs_dir2.h b/libxfs/xfs_dir2.h
> index 89b9e24..ea20c12 100644
> --- a/libxfs/xfs_dir2.h
> +++ b/libxfs/xfs_dir2.h
> @@ -32,10 +32,9 @@ struct xfs_dir2_data_unused;
> extern struct xfs_name xfs_name_dotdot;
>
> /*
> - * directory filetype conversion tables.
> + * Convert inode mode to directory entry filetype
> */
> -#define S_SHIFT 12
> -extern const unsigned char xfs_mode_to_ftype[];
> +extern const unsigned char xfs_mode_to_ftype(int mode);
>
> /*
> * directory operations vector for encode/decode routines
> diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
> index 4db9512..a3909ac 100644
> --- a/repair/dino_chunks.c
> +++ b/repair/dino_chunks.c
> @@ -826,8 +826,6 @@ next_readbuf:
> * we do now, this is where to start.
> */
> if (is_used) {
> - __uint16_t di_mode;
> -
> if (is_inode_free(ino_rec, irec_offset)) {
> if (verbose || no_modify) {
> do_warn(
> @@ -846,10 +844,8 @@ next_readbuf:
> * store the on-disk file type for comparing in
> * phase 6.
> */
> - di_mode = be16_to_cpu(dino->di_mode);
> - di_mode = (di_mode & S_IFMT) >> S_SHIFT;
> set_inode_ftype(ino_rec, irec_offset,
> - xfs_mode_to_ftype[di_mode]);
> + xfs_mode_to_ftype(be16_to_cpu(dino->di_mode)));
>
> /*
> * store on-disk nlink count for comparing in phase 7
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 06eed16..c492dd9 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -1082,7 +1082,7 @@ mv_orphanage(
> if ((err = -libxfs_iget(mp, NULL, ino, 0, &ino_p)))
> do_error(_("%d - couldn't iget disconnected inode\n"), err);
>
> - xname.type = xfs_mode_to_ftype[(VFS_I(ino_p)->i_mode & S_IFMT)>>S_SHIFT];
> + xname.type = xfs_mode_to_ftype(VFS_I(ino_p)->i_mode);
>
> if (isa_dir) {
> irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, orphanage_ino),
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement
2017-01-11 16:23 ` Eric Sandeen
@ 2017-01-20 10:58 ` Amir Goldstein
2017-01-20 20:33 ` Eric Sandeen
0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2017-01-20 10:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Darrick J . Wong, Brian Foster, Christoph Hellwig, linux-xfs
On Wed, Jan 11, 2017 at 6:23 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
> On 1/11/17 12:30 AM, Amir Goldstein wrote:
>> The size of the xfs_mode_to_ftype[] conversion table
>> was too small to handle an invalid value of mode=S_IFMT.
>>
>> Instead of fixing the table size, replace the conversion table
>> with a conversion helper that uses a switch statement.
>>
>> Suggested-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>> libxfs/xfs_dir2.c | 36 ++++++++++++++++++++++--------------
>> libxfs/xfs_dir2.h | 5 ++---
>> repair/dino_chunks.c | 6 +-----
>> repair/phase6.c | 2 +-
>> 4 files changed, 26 insertions(+), 23 deletions(-)
>>
>> Eric,
>>
>> This is a matching patch to the one that got Reviewed-by Darrick for kernel.
>>
>> Tested xfs_repair with the new xfs/348 test with -n ftype=0|1.
>
> Thanks, I'll pull this in after Darrick commits it to the kernel.
>
FYI, commit is in kernel and xfstest xfs/348 which sanity tests
xfs_repair with this change
is already merged as well.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement
2017-01-20 10:58 ` Amir Goldstein
@ 2017-01-20 20:33 ` Eric Sandeen
0 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2017-01-20 20:33 UTC (permalink / raw)
To: Amir Goldstein
Cc: Darrick J . Wong, Brian Foster, Christoph Hellwig, linux-xfs
On 1/20/17 4:58 AM, Amir Goldstein wrote:
> On Wed, Jan 11, 2017 at 6:23 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
>> On 1/11/17 12:30 AM, Amir Goldstein wrote:
>>> The size of the xfs_mode_to_ftype[] conversion table
>>> was too small to handle an invalid value of mode=S_IFMT.
>>>
>>> Instead of fixing the table size, replace the conversion table
>>> with a conversion helper that uses a switch statement.
>>>
>>> Suggested-by: Christoph Hellwig <hch@lst.de>
>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>> ---
>>> libxfs/xfs_dir2.c | 36 ++++++++++++++++++++++--------------
>>> libxfs/xfs_dir2.h | 5 ++---
>>> repair/dino_chunks.c | 6 +-----
>>> repair/phase6.c | 2 +-
>>> 4 files changed, 26 insertions(+), 23 deletions(-)
>>>
>>> Eric,
>>>
>>> This is a matching patch to the one that got Reviewed-by Darrick for kernel.
>>>
>>> Tested xfs_repair with the new xfs/348 test with -n ftype=0|1.
>>
>> Thanks, I'll pull this in after Darrick commits it to the kernel.
>>
>
> FYI, commit is in kernel and xfstest xfs/348 which sanity tests
> xfs_repair with this change
> is already merged as well.
Yep, merged & testing now.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-20 20:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 6:30 [PATCH v3] xfsprogs: replace xfs_mode_to_ftype table with switch statement Amir Goldstein
2017-01-11 16:23 ` Eric Sandeen
2017-01-20 10:58 ` Amir Goldstein
2017-01-20 20:33 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.