Re: [libvirt PATCH v2 10/11] qemuMonitorGetChardevInfo: Use automatic memory management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux