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(-)

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;
     }
 
-- 
2.17.0

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?