Attention is currently required from: Jakub Czapiga, Julius Werner, Philipp Hug, ron minnich.

Nico Huber has uploaded this change for review.

View Change

[RFC] region: Hide struct region members

We cannot make `struct region` opaque for several reasons (size needs
to be known for local objects, a lot of inline API functions, ...).
However, we still want to encourage using the high-level region API
instead of directly accessing and manipulating the struct members.
This patch tries to achieve this by renaming the struct members. Only
if REGION_INTERNAL_STRUCTURES is defined before including <region.h>,
the usual member names are used.

Change-Id: I713be9cf0bab4c2e21113b55e7229ab50f06c6cf
Signed-off-by: Nico Huber <nico.h@gmx.de>
---
M src/arch/arm/fit_payload.c
M src/arch/arm64/fit_payload.c
M src/arch/riscv/fit_payload.c
M src/commonlib/include/commonlib/region.h
M src/commonlib/region.c
M src/lib/fit_payload.c
M tests/commonlib/region-test.c
M tests/lib/cbfs-lookup-test.c
M tests/lib/fmap-test.c
9 files changed, 26 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/79907/1

diff --git a/src/arch/arm/fit_payload.c b/src/arch/arm/fit_payload.c
index 19f46a8..84c6dbb 100644
--- a/src/arch/arm/fit_payload.c
+++ b/src/arch/arm/fit_payload.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#define REGION_INTERNAL_STRUCTURES /* FIXME: should use the high-level region api */
 #include <bootmem.h>
 #include <program_loading.h>
 #include <fit.h>
diff --git a/src/arch/arm64/fit_payload.c b/src/arch/arm64/fit_payload.c
index f8ae16a..f6d6386 100644
--- a/src/arch/arm64/fit_payload.c
+++ b/src/arch/arm64/fit_payload.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#define REGION_INTERNAL_STRUCTURES /* FIXME: should use the high-level region api */
 #include <commonlib/bsd/compression.h>
 #include <console/console.h>
 #include <bootmem.h>
diff --git a/src/arch/riscv/fit_payload.c b/src/arch/riscv/fit_payload.c
index bfa0c98..c879b9e 100644
--- a/src/arch/riscv/fit_payload.c
+++ b/src/arch/riscv/fit_payload.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#define REGION_INTERNAL_STRUCTURES /* FIXME: should use the high-level region api */
 #include <commonlib/bsd/compression.h>
 #include <console/console.h>
 #include <bootmem.h>
diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h
index a730c58..4fdbb80 100644
--- a/src/commonlib/include/commonlib/region.h
+++ b/src/commonlib/include/commonlib/region.h
@@ -73,9 +73,17 @@
 	ssize_t (*eraseat)(const struct region_device *, size_t, size_t);
 };
 
+#ifdef REGION_INTERNAL_STRUCTURES
+#define _region_offset offset
+#define _region_size size
+#else
+#define _region_offset _region_internal_only__offset
+#define _region_size _region_internal_only__size
+#endif
+
 struct region {
-	size_t offset;
-	size_t size;
+	size_t _region_offset;
+	size_t _region_size;
 };
 
 struct region_device {
@@ -89,15 +97,15 @@
 		.root = NULL,				\
 		.ops = (ops_),				\
 		.region = {				\
-			.offset = (offset_),		\
-			.size = (size_),		\
+			(offset_),			\
+			(size_),			\
 		},					\
 	}
 
 static inline struct region region_create(size_t offset, size_t size)
 {
 	assert(offset + size - 1 >= offset);
-	return (struct region){ .offset = offset, .size = size };
+	return (struct region){ ._region_offset = offset, ._region_size = size };
 }
 
 static inline int region_create_untrusted(
@@ -114,14 +122,17 @@
 
 static inline size_t region_offset(const struct region *r)
 {
-	return r->offset;
+	return r->_region_offset;
 }
 
 static inline size_t region_sz(const struct region *r)
 {
-	return r->size;
+	return r->_region_size;
 }
 
+#undef _region_offset
+#undef _region_size
+
 static inline unsigned long long region_end(const struct region *r)
 {
 	return region_offset(r) + region_sz(r);
diff --git a/src/commonlib/region.c b/src/commonlib/region.c
index 4153f0a..96c70fa 100644
--- a/src/commonlib/region.c
+++ b/src/commonlib/region.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#define REGION_INTERNAL_STRUCTURES
 #include <commonlib/helpers.h>
 #include <commonlib/region.h>
 #include <stdint.h>
diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c
index 21bc4e8..64ab4fb 100644
--- a/src/lib/fit_payload.c
+++ b/src/lib/fit_payload.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#define REGION_INTERNAL_STRUCTURES /* FIXME: should use the high-level region api */
 #include <commonlib/bsd/compression.h>
 #include <console/console.h>
 #include <bootmem.h>
diff --git a/tests/commonlib/region-test.c b/tests/commonlib/region-test.c
index 3280482..fd82fa6 100644
--- a/tests/commonlib/region-test.c
+++ b/tests/commonlib/region-test.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#define REGION_INTERNAL_STRUCTURES
 #include <commonlib/region.h>
 #include <string.h>
 #include <tests/test.h>
diff --git a/tests/lib/cbfs-lookup-test.c b/tests/lib/cbfs-lookup-test.c
index 7562868..8e46e73 100644
--- a/tests/lib/cbfs-lookup-test.c
+++ b/tests/lib/cbfs-lookup-test.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#define REGION_INTERNAL_STRUCTURES
 #include <assert.h>
 #include <cbfs.h>
 #include <commonlib/bsd/cbfs_mdata.h>
diff --git a/tests/lib/fmap-test.c b/tests/lib/fmap-test.c
index 1cc8246..43c046f 100644
--- a/tests/lib/fmap-test.c
+++ b/tests/lib/fmap-test.c
@@ -5,6 +5,7 @@
 
 #include <tests/test.h>
 
+#define REGION_INTERNAL_STRUCTURES
 #include <fmap.h>
 #include <commonlib/region.h>
 

To view, visit change 79907. To unsubscribe, or for help writing mail filters, visit settings.