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@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