[PATCH v1] Refactor out some I/O logic into separate function

Kelvin Zhang zhangkelvin at google.com
Wed Nov 24 11:26:13 AEDT 2021


Many of the global variables are for I/O purposes. To make the codebase
more library friendly, decouple I/O and parsing into separate functions.

Change-Id: I7dbb3dcd6e82b075205891f9a06b80e1191fa5d6
Signed-off-by: Kelvin Zhang <zhangkelvin at google.com>
---
 include/erofs/defs.h  | 19 ++++++++++++
 include/erofs/parse.h | 23 +++++++++++++++
 include/erofs_fs.h    | 12 ++++++++
 lib/io.c              |  1 +
 lib/namei.c           | 34 +++++++++++++++-------
 lib/super.c           | 67 ++++++++++++++++++++++++++-----------------
 6 files changed, 118 insertions(+), 38 deletions(-)
 create mode 100644 include/erofs/parse.h

diff --git a/include/erofs/defs.h b/include/erofs/defs.h
index 6398cbb..16376dc 100644
--- a/include/erofs/defs.h
+++ b/include/erofs/defs.h
@@ -8,6 +8,11 @@
 #ifndef __EROFS_DEFS_H
 #define __EROFS_DEFS_H
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 #include <stddef.h>
 #include <stdint.h>
 #include <assert.h>
@@ -82,12 +87,18 @@ typedef int64_t         s64;
 #endif
 #endif
 
+#ifdef __cplusplus
+#define BUILD_BUG_ON(condition) static_assert(!condition)
+#else
+
 #ifndef __OPTIMIZE__
 #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2 * !!(condition)]))
 #else
 #define BUILD_BUG_ON(condition) assert(!(condition))
 #endif
 
+#endif
+
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define __round_mask(x, y)      ((__typeof__(x))((y)-1))
@@ -110,6 +121,8 @@ typedef int64_t         s64;
 }							\
 )
 
+// Defining min/max macros in C++ will cause conflicts with std::min/max
+#ifndef __cplusplus
 #define min(x, y) ({				\
 	typeof(x) _min1 = (x);			\
 	typeof(y) _min2 = (y);			\
@@ -121,6 +134,7 @@ typedef int64_t         s64;
 	typeof(y) _max2 = (y);			\
 	(void) (&_max1 == &_max2);		\
 	_max1 > _max2 ? _max1 : _max2; })
+#endif
 
 /*
  * ..and if you can't take the strict types, you can specify one yourself.
@@ -308,4 +322,9 @@ unsigned long __roundup_pow_of_two(unsigned long n)
 #define stat64		stat
 #define lstat64		lstat
 #endif
+
+#ifdef __cplusplus
+}
+#endif
+
 #endif
diff --git a/include/erofs/parse.h b/include/erofs/parse.h
new file mode 100644
index 0000000..65948a1
--- /dev/null
+++ b/include/erofs/parse.h
@@ -0,0 +1,23 @@
+
+#ifndef __EROFS_PARSE_H
+#define __EROFS_PARSE_H
+
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
+#include "erofs_fs.h"
+#include "internal.h"
+
+int erofs_parse_inode_from_buffer(const char buf[EROFS_MAX_INODE_SIZE],
+                                  struct erofs_inode *vi);
+
+int erofs_parse_superblock(const char data[EROFS_BLKSIZ],
+                           struct erofs_sb_info *sbi);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/include/erofs_fs.h b/include/erofs_fs.h
index 9a91877..3f1f645 100644
--- a/include/erofs_fs.h
+++ b/include/erofs_fs.h
@@ -9,6 +9,12 @@
 #ifndef __EROFS_FS_H
 #define __EROFS_FS_H
 
+#ifdef __cplusplus
+#define inline constexpr
+extern "C"
+{
+#endif
+
 #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
 #define EROFS_SUPER_OFFSET      1024
 
@@ -189,6 +195,8 @@ struct erofs_inode_extended {
 	__u8   i_reserved2[16];
 };
 
+#define EROFS_MAX_INODE_SIZE sizeof(struct erofs_inode_extended)
+
 #define EROFS_MAX_SHARED_XATTRS         (128)
 /* h_shared_count between 129 ... 255 are special # */
 #define EROFS_SHARED_XATTR_EXTENT       (255)
@@ -425,4 +433,8 @@ static inline void erofs_check_ondisk_layout_definitions(void)
 		     Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
 }
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif
diff --git a/lib/io.c b/lib/io.c
index a0d366a..9ee754b 100644
--- a/lib/io.c
+++ b/lib/io.c
@@ -12,6 +12,7 @@
 #endif
 #include <sys/stat.h>
 #include <sys/ioctl.h>
+#include <sys/fcntl.h>
 #include "erofs/io.h"
 #ifdef HAVE_LINUX_FS_H
 #include <linux/fs.h>
diff --git a/lib/namei.c b/lib/namei.c
index 7377e74..95c379c 100644
--- a/lib/namei.c
+++ b/lib/namei.c
@@ -22,18 +22,15 @@ static dev_t erofs_new_decode_dev(u32 dev)
 	return makedev(major, minor);
 }
 
-int erofs_read_inode_from_disk(struct erofs_inode *vi)
+int erofs_parse_inode_from_buffer(
+	const char buf[EROFS_MAX_INODE_SIZE],
+	struct erofs_inode *vi)
 {
 	int ret, ifmt;
-	char buf[sizeof(struct erofs_inode_extended)];
 	struct erofs_inode_compact *dic;
 	struct erofs_inode_extended *die;
 	const erofs_off_t inode_loc = iloc(vi->nid);
 
-	ret = dev_read(0, buf, inode_loc, sizeof(*dic));
-	if (ret < 0)
-		return -EIO;
-
 	dic = (struct erofs_inode_compact *)buf;
 	ifmt = le16_to_cpu(dic->i_format);
 
@@ -47,11 +44,6 @@ int erofs_read_inode_from_disk(struct erofs_inode *vi)
 	case EROFS_INODE_LAYOUT_EXTENDED:
 		vi->inode_isize = sizeof(struct erofs_inode_extended);
 
-		ret = dev_read(0, buf + sizeof(*dic), inode_loc + sizeof(*dic),
-			       sizeof(*die) - sizeof(*dic));
-		if (ret < 0)
-			return -EIO;
-
 		die = (struct erofs_inode_extended *)buf;
 		vi->xattr_isize = erofs_xattr_ibody_size(die->i_xattr_icount);
 		vi->i_mode = le16_to_cpu(die->i_mode);
@@ -144,6 +136,26 @@ bogusimode:
 	return -EFSCORRUPTED;
 }
 
+int erofs_read_inode_from_disk(struct erofs_inode *vi)
+{
+	char buf[sizeof(struct erofs_inode_extended)];
+	struct erofs_inode_compact *dic;
+	struct erofs_inode_extended *die;
+	const erofs_off_t inode_loc = iloc(vi->nid);
+
+	int ret = dev_read(0, buf, inode_loc, sizeof(*dic));
+	if (ret < 0)
+		return -EIO;
+	int ifmt = le16_to_cpu(dic->i_format);
+	if (erofs_inode_version(ifmt)) {
+		ret = dev_read(0, buf + sizeof(*dic), inode_loc + sizeof(*dic),
+			       sizeof(*die) - sizeof(*dic));
+		if (ret < 0)
+			return -EIO;	
+	}
+	return erofs_parse_inode_from_buffer(buf, vi);
+}
+
 
 struct erofs_dirent *find_target_dirent(erofs_nid_t pnid,
 					void *dentry_blk,
diff --git a/lib/super.c b/lib/super.c
index 3ccc551..0e26992 100644
--- a/lib/super.c
+++ b/lib/super.c
@@ -4,6 +4,7 @@
  */
 #include <string.h>
 #include <stdlib.h>
+#include "erofs/internal.h"
 #include "erofs/io.h"
 #include "erofs/print.h"
 
@@ -62,29 +63,21 @@ static int erofs_init_devices(struct erofs_sb_info *sbi,
 	return 0;
 }
 
-int erofs_read_superblock(void)
-{
-	char data[EROFS_BLKSIZ];
-	struct erofs_super_block *dsb;
-	unsigned int blkszbits;
-	int ret;
+int erofs_parse_superblock(
+	const char data[EROFS_BLKSIZ],
+	struct erofs_sb_info *sbi
+) {
+	struct erofs_super_block *dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);
 
-	ret = blk_read(0, data, 0, 1);
-	if (ret < 0) {
-		erofs_err("cannot read erofs superblock: %d", ret);
-		return -EIO;
-	}
-	dsb = (struct erofs_super_block *)(data + EROFS_SUPER_OFFSET);
-
-	ret = -EINVAL;
+	int ret = -EINVAL;
 	if (le32_to_cpu(dsb->magic) != EROFS_SUPER_MAGIC_V1) {
 		erofs_err("cannot find valid erofs superblock");
 		return ret;
 	}
 
-	sbi.feature_compat = le32_to_cpu(dsb->feature_compat);
+	sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
 
-	blkszbits = dsb->blkszbits;
+	unsigned int blkszbits = dsb->blkszbits;
 	/* 9(512 bytes) + LOG_SECTORS_PER_BLOCK == LOG_BLOCK_SIZE */
 	if (blkszbits != LOG_BLOCK_SIZE) {
 		erofs_err("blksize %u isn't supported on this platform",
@@ -92,20 +85,40 @@ int erofs_read_superblock(void)
 		return ret;
 	}
 
-	if (!check_layout_compatibility(&sbi, dsb))
+	if (!check_layout_compatibility(sbi, dsb))
 		return ret;
 
-	sbi.primarydevice_blocks = le32_to_cpu(dsb->blocks);
-	sbi.meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
-	sbi.xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
-	sbi.islotbits = EROFS_ISLOTBITS;
-	sbi.root_nid = le16_to_cpu(dsb->root_nid);
-	sbi.inos = le64_to_cpu(dsb->inos);
-	sbi.checksum = le32_to_cpu(dsb->checksum);
+	sbi->primarydevice_blocks = le32_to_cpu(dsb->blocks);
+	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
+	sbi->xattr_blkaddr = le32_to_cpu(dsb->xattr_blkaddr);
+	sbi->islotbits = EROFS_ISLOTBITS;
+	sbi->root_nid = le16_to_cpu(dsb->root_nid);
+	sbi->inos = le64_to_cpu(dsb->inos);
+	sbi->checksum = le32_to_cpu(dsb->checksum);
 
-	sbi.build_time = le64_to_cpu(dsb->build_time);
-	sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
+	sbi->build_time = le64_to_cpu(dsb->build_time);
+	sbi->build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
+
+	memcpy(&sbi->uuid, dsb->uuid, sizeof(dsb->uuid));
+	return 0;
+}
 
-	memcpy(&sbi.uuid, dsb->uuid, sizeof(dsb->uuid));
+int erofs_read_superblock(void)
+{
+	char data[EROFS_BLKSIZ];
+	struct erofs_super_block *dsb;
+	unsigned int blkszbits;
+	int ret;
+
+	ret = blk_read(0, data, 0, 1);
+	if (ret < 0) {
+		erofs_err("cannot read erofs superblock: %d", ret);
+		return -EIO;
+	}
+	ret = erofs_parse_superblock(data, &sbi);
+	if (ret) {
+		return ret;
+	}
+	
 	return erofs_init_devices(&sbi, dsb);
 }
-- 
2.34.0.rc2.393.gf8c9666880-goog



More information about the Linux-erofs mailing list