| Message ID | 20221202030003.11441-8-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce xenpv machine for arm architecture | expand |
On 2/12/22 03:59, Vikram Garhwal wrote: > Replace g_malloc with g_new and perror with error_setg_errno. > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > --- > hw/xen/xen-hvm-common.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > @@ -717,7 +717,7 @@ void destroy_hvm_domain(bool reboot) > xc_interface *xc_handle; > int sts; > int rc; > - > + Error *errp = NULL; > unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff; > > if (xen_dmod) { > @@ -726,7 +726,7 @@ void destroy_hvm_domain(bool reboot) > return; > } > if (errno != ENOTTY /* old Xen */) { > - perror("xendevicemodel_shutdown failed"); > + error_setg_errno(&errp, errno, "xendevicemodel_shutdown failed"); See "qapi/error.h": * = Passing errors around = * * Errors get passed to the caller through the conventional @errp * parameter. Here you are not passing the error to the caller. Maybe you are looking for the "qemu/error-report.h" API? > } > /* well, try the old thing then */ > } > @@ -857,16 +857,17 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, > MemoryListener xen_memory_listener) > { > int rc; > + Error *errp = NULL; > > state->xce_handle = xenevtchn_open(NULL, 0); > if (state->xce_handle == NULL) { > - perror("xen: event channel open"); > + error_setg_errno(&errp, errno, "xen: event channel open"); > goto err; > } > > state->xenstore = xs_daemon_open(); > if (state->xenstore == NULL) { > - perror("xen: xenstore open"); > + error_setg_errno(&errp, errno, "xen: xenstore open"); > goto err; > } > Ditto.
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 2/12/22 03:59, Vikram Garhwal wrote: >> Replace g_malloc with g_new and perror with error_setg_errno. >> >> Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> >> --- >> hw/xen/xen-hvm-common.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) > > >> @@ -717,7 +717,7 @@ void destroy_hvm_domain(bool reboot) >> xc_interface *xc_handle; >> int sts; >> int rc; >> - >> + Error *errp = NULL; >> unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff; >> >> if (xen_dmod) { >> @@ -726,7 +726,7 @@ void destroy_hvm_domain(bool reboot) >> return; >> } >> if (errno != ENOTTY /* old Xen */) { >> - perror("xendevicemodel_shutdown failed"); >> + error_setg_errno(&errp, errno, "xendevicemodel_shutdown failed"); > > See "qapi/error.h": > > * = Passing errors around = > * > * Errors get passed to the caller through the conventional @errp > * parameter. > > Here you are not passing the error to the caller. Instead, you're leaking its memory. > Maybe you are looking for the "qemu/error-report.h" API? Plausible. Also, @errp is the conventional name for the Error ** parameter used to pass errors to the caller. Local Error * variables are usually called @err or @local_err (I prefer the former). [...]
Hi Markus & Philippe, Thanks for reviewing this one. Please see the question below. On 12/2/22 12:53 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> On 2/12/22 03:59, Vikram Garhwal wrote: >>> Replace g_malloc with g_new and perror with error_setg_errno. >>> >>> Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> >>> --- >>> hw/xen/xen-hvm-common.c | 15 ++++++++------- >>> 1 file changed, 8 insertions(+), 7 deletions(-) >> >>> @@ -717,7 +717,7 @@ void destroy_hvm_domain(bool reboot) >>> xc_interface *xc_handle; >>> int sts; >>> int rc; >>> - >>> + Error *errp = NULL; >>> unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff; >>> >>> if (xen_dmod) { >>> @@ -726,7 +726,7 @@ void destroy_hvm_domain(bool reboot) >>> return; >>> } >>> if (errno != ENOTTY /* old Xen */) { >>> - perror("xendevicemodel_shutdown failed"); >>> + error_setg_errno(&errp, errno, "xendevicemodel_shutdown failed"); >> See "qapi/error.h": >> >> * = Passing errors around = >> * >> * Errors get passed to the caller through the conventional @errp >> * parameter. >> >> Here you are not passing the error to the caller. > Instead, you're leaking its memory. > >> Maybe you are looking for the "qemu/error-report.h" API? > Plausible. > > Also, @errp is the conventional name for the Error ** parameter used to > pass errors to the caller. Local Error * variables are usually called > @err or @local_err (I prefer the former). > > [...] > IIUC, error_set_errno() is not okay as it needs to be called with errp from caller. But error_set(&err, "") is okay with locally defined **err = NULL; Is that correct understanding?
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index 03128e575b..4ba5141fa2 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -34,7 +34,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, trace_xen_ram_alloc(ram_addr, size); nr_pfn = size >> TARGET_PAGE_BITS; - pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn); + pfn_list = g_new(xen_pfn_t, nr_pfn); for (i = 0; i < nr_pfn; i++) { pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i; @@ -717,7 +717,7 @@ void destroy_hvm_domain(bool reboot) xc_interface *xc_handle; int sts; int rc; - + Error *errp = NULL; unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff; if (xen_dmod) { @@ -726,7 +726,7 @@ void destroy_hvm_domain(bool reboot) return; } if (errno != ENOTTY /* old Xen */) { - perror("xendevicemodel_shutdown failed"); + error_setg_errno(&errp, errno, "xendevicemodel_shutdown failed"); } /* well, try the old thing then */ } @@ -797,7 +797,7 @@ static void xen_do_ioreq_register(XenIOState *state, } /* Note: cpus is empty at this point in init */ - state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *)); + state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus); rc = xen_set_ioreq_server_state(xen_domid, state->ioservid, true); if (rc < 0) { @@ -806,7 +806,7 @@ static void xen_do_ioreq_register(XenIOState *state, goto err; } - state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t)); + state->ioreq_local_port = g_new0(evtchn_port_t, max_cpus); /* FIXME: how about if we overflow the page here? */ for (i = 0; i < max_cpus; i++) { @@ -857,16 +857,17 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, MemoryListener xen_memory_listener) { int rc; + Error *errp = NULL; state->xce_handle = xenevtchn_open(NULL, 0); if (state->xce_handle == NULL) { - perror("xen: event channel open"); + error_setg_errno(&errp, errno, "xen: event channel open"); goto err; } state->xenstore = xs_daemon_open(); if (state->xenstore == NULL) { - perror("xen: xenstore open"); + error_setg_errno(&errp, errno, "xen: xenstore open"); goto err; }
Replace g_malloc with g_new and perror with error_setg_errno. Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> --- hw/xen/xen-hvm-common.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)