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