Re: [QEMU][PATCH v2 07/11] hw/xen/xen-hvm-common: Use g_new and error_setg_errno

[Vikram Garhwal]

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?