Bibo Mao <maob...@loongson.cn> writes:
> There is NULL pointer checking function error_propagate() already,
> it is not necessary to add checking for function parameter. Here remove
> NULL pointer checking with function parameter.
>
> Signed-off-by: Bibo Mao <maob...@loongson.cn>
> ---
> hw/loongarch/virt.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index a5840ff968..d82676d316 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -868,21 +868,24 @@ 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;
Make this
error_setg(&err, ...);
return;
> }
>
> if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
> error_setg(&err,
> "Invalid core-id %u specified, must be in range 1:%u",
> cpu->core_id, ms->smp.cores - 1);
> - goto out;
> + error_propagate(errp, err);
> + return;
Likewise.
> }
>
> if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
> error_setg(&err,
> "Invalid socket-id %u specified, must be in range
> 1:%u",
> cpu->socket_id, ms->smp.sockets - 1);
> - goto out;
> + error_propagate(errp, err);
> + return;
Likewise.
> }
>
> topo.socket_id = cpu->socket_id;
> @@ -895,7 +898,8 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
> "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
> cs->cpu_index, cpu->socket_id, cpu->core_id,
> cpu->thread_id, cpu_slot->arch_id);
> - goto out;
> + error_propagate(errp, err);
> + return;
Likewise.
> }
> } else {
> /* For cold-add cpu, find empty cpu slot */
> @@ -912,10 +916,6 @@ static void virt_cpu_pre_plug(HotplugHandler
> *hotplug_dev,
> cpu->phy_id = cpu_slot->arch_id;
> cs->cpu_index = cpu_slot - ms->possible_cpus->cpus;
> numa_cpu_pre_plug(cpu_slot, dev, &err);
You need to pass errp instead of &err now.
> -out:
> - if (err) {
> - error_propagate(errp, err);
> - }
> }
>
> static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
> @@ -935,9 +935,7 @@ static void virt_cpu_unplug_request(HotplugHandler
> *hotplug_dev,
> }
>
> hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev,
> &err);
> - if (err) {
> - error_propagate(errp, err);
> - }
> + error_propagate(errp, err);
> }
Correct, but I'd recomment to go one step further:
static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
- Error *err = NULL;
LoongArchCPU *cpu = LOONGARCH_CPU(dev);
CPUState *cs = CPU(dev);
if (cs->cpu_index == 0) {
- error_setg(&err, "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
+ error_setg(errp, "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
cs->cpu_index, cpu->socket_id,
cpu->core_id, cpu->thread_id);
- error_propagate(errp, err);
return;
}
- hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
- if (err) {
- error_propagate(errp, err);
- }
+ hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, errp);
}
>
> static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
> @@ -1001,9 +999,8 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
cpu_slot->cpu = CPU(dev);
if (lvms->ipi) {
hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev, &err);
if (err) {
error_propagate(errp, err);
return;
}
}
if (lvms->extioi) {
hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), dev, &err);
if (err) {
error_propagate(errp, err);
return;
}
}
>
> if (lvms->acpi_ged) {
> hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
> - if (err) {
> - error_propagate(errp, err);
> - }
> + error_propagate(errp, err);
> + return;
> }
>
> return;
Better make this work exactly like the other checks above, and drop the
final return, which serves no purpose:
if (err) {
error_propagate(errp, err);
return;
}
}
}
