Attention is currently required from: Jakub Czapiga, Julius Werner, Philipp Hug, ron minnich.
Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79907?usp=email )
Change subject: [RFC] region: Hide struct region members ......................................................................
[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>