bibo mao <maob...@loongson.cn> writes:
On 2025/3/13 下午6:32, Markus Armbruster wrote:
[...]
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..4674bd9163 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -859,30 +859,29 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
LoongArchCPU *cpu = LOONGARCH_CPU(dev);
CPUState *cs = CPU(dev);
CPUArchId *cpu_slot;
- Error *err = NULL;
LoongArchCPUTopo topo;
int arch_id;
if (lvms->acpi_ged) {
if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
- error_setg(&err,
+ error_setg(errp,
"Invalid thread-id %u specified, must be in range
1:%u",
cpu->thread_id, ms->smp.threads - 1);
- goto out;
+ return;
Hi Markus,
From APIs, it seems that function error_propagate() do much more than
error appending, such as comparing dest_err with error_abort etc. Though
caller function is local variable rather than error_abort/fatal/warn,
error_propagate() seems useful. How about do propagate error and return
directly as following:
@@ -868,7 +868,8 @@ static void virt_cpu_pre_plug(HotplugHandler
*hotplug_dev,
error_setg(&err,
"Invalid thread-id %u specified, must be in
range 1:%u",
cpu->thread_id, ms->smp.threads - 1);
- goto out;
+ error_propagate(errp, err);
+ return;
}
This is strictly worse. One, it's more verbose. Two, the stack
backtrace on failure is less useful, which matters when @errp is
&error_abort, and when you set a breakpoint on error_handle(), abort(),
or exit(). Three, it doesn't actually add useful functionality.
To help you see the latter, let's compare the two versions, i.e.
direct: error_setg(errp, ...)
and
propagate: two steps, first error_setg(&err, ...), and then
error_propagate(errp, err);
Cases: @errp can be NULL, &error_abort, &error_fatal, &error_warn, or a
non-null pointer to variable containing NULL.
1. @errp is NULL
Direct does nothing.
Propagate step 1 creates an error object, and stores it in @err.
Step 2 frees the error object. Roundabout way to do nothing.
2. @errp is &error_abort
Direct creates an error object, reports it to stderr, and abort()s.
Note that the stack backtrace shows where the error is created.
Propagate step 1 creates an error object, and stores it in @err.
Step 2 reports it to stderr, and abort()s. No difference, except the
stack backtrace shows where the error is propagated, which is less
useful.
3. @errp is &error_fatal
Direct creates an error object, reports it to stderr, frees it, and
exit(1)s.
Propagate step 1 creates an error object, and stores it in @err.
Step 2 reports it to stderr, frees it, and exit(1)s. No difference.
4. @errp is &error_warn
Direct creates an error object, reports it to stderr, and frees it.
Propagate step 1 creates an error object, and stores it in @err.
Step 2 reports it to stderr, and frees it. No difference.
5. @errp points to variable containing NULL
Direct creates an error object, and stores it in the variable.
Propagate step 1 creates an error object, and stores it in @err.
Step 2 copies it to the variable. No difference.
Questions?
