On 2026/03/04 20:38, BALATON Zoltan wrote:
On Wed, 4 Mar 2026, Akihiko Odaki wrote:
On 2026/03/04 6:47, BALATON Zoltan wrote:
Introduce internal helper functions to remove duplicated code from
different memory_region_init_*ram functions.
Signed-off-by: BALATON Zoltan <[email protected]>
---
system/memory.c | 132 ++++++++++++++++++------------------------------
1 file changed, 50 insertions(+), 82 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index e8c4912a60..1b26c6b5a5 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1589,19 +1589,16 @@ bool
memory_region_init_ram_nomigrate(MemoryRegion *mr,
size, 0, errp);
}
-bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- uint32_t ram_flags,
- Error **errp)
+static void memory_region_setup_ram(MemoryRegion *mr)
{
- Error *err = NULL;
- memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
- mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
+}
+
+static bool memory_region_error_propagate(MemoryRegion *mr,
+ Error *err, Error **errp)
+{
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
@@ -1611,6 +1608,18 @@ bool
memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
return true;
}
I think the optimal way to factor out error propagation is to use
ERRP_GUARD().
I don't think ERRP_GUARD applies here as we do not dereference errp and
this does more than just propagate the error so I don't see how
ERRP_GUARD could help here.
include/qapi/error.h has an extensive documentation describing how
errors should be passed around:
> Call a function, receive an error from it, and pass it to the caller
> - when the function returns a value that indicates failure, say
> false:
> if (!foo(arg, errp)) {
> handle the error...
> }
> - when it does not, say because it is a void function:
> ERRP_GUARD();
> foo(arg, errp);
> if (*errp) {
> handle the error...
> }
> More on ERRP_GUARD() below.
>
> Code predating ERRP_GUARD() still exists, and looks like this:
> Error *err = NULL;
> foo(arg, &err);
> if (err) {
> handle the error...
> error_propagate(errp, err); // deprecated
> }
> Avoid in new code. Do *not* "optimize" it to
> foo(arg, errp);
> if (*errp) { // WRONG!
> handle the error...
> }
> because errp may be NULL without the ERRP_GUARD() guard.
>
> But when all you do with the error is pass it on, please use
> foo(arg, errp);
> for readability.
>
> Receive an error, and handle it locally
> - when the function returns a value that indicates failure, say
> false:
> Error *err = NULL;
> if (!foo(arg, &err)) {
> handle the error...
> }
> - when it does not, say because it is a void function:
> Error *err = NULL;
> foo(arg, &err);
> if (err) {
> handle the error...
> }
>
> Pass an existing error to the caller:
> error_propagate(errp, err);
> This is rarely needed. When @err is a local variable, use of
> ERRP_GUARD() commonly results in more readable code.
But looking at the code, the functions generating errors (e.g.,
qemu_ram_alloc()) return values that indicate failures (NULL), so I now
think we should use the first pattern I cited (i.e., check the returned
value instead of err) and remove the err variable and error_propagate()
altogether instead of factoring them out with
memory_region_error_propagate() or ERRP_GUARD().
Regards,
Akihiko Odaki
