On Tue, 2021-07-13 at 09:36 +0200, Michal Prívozník wrote:
On 7/6/21 2:37 PM, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
> ---
> src/qemu/qemu_monitor.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index cb59fc7b7b..4489b809f4 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2884,25 +2884,21 @@ int
> qemuMonitorGetChardevInfo(qemuMonitor *mon,
> GHashTable **retinfo)
> {
> - GHashTable *info = NULL;
> + g_autoptr(GHashTable) info = NULL;
>
> VIR_DEBUG("retinfo=%p", retinfo);
>
> - QEMU_CHECK_MONITOR_GOTO(mon, error);
> + QEMU_CHECK_MONITOR(mon);
>
> + *retinfo = NULL;
This feels redundant. In previous patches you changed the code so
that
the output argument is set only in case of success. I think this line
should be removed.
Previous behavior was to set *retinfo to NULL explicitly ...
> if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
> - goto error;
> + return -1;
>
> if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
> - goto error;
> + return -1;
>
> - *retinfo = info;
> + *retinfo = g_steal_pointer(&info);
> return 0;
> -
> - error:
> - virHashFree(info);
> - *retinfo = NULL;
... here, if an error occured. From what I can tell, this is not really
neccessary, feel free to merge with or without the explicit "*retinfo =
NULL;".
Thanks,
Tim
> - return -1;
> }
>
>
>
Michal