Re: [PATCH] Fix a reference leak for node devices.

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

 



On 06/25/10 - 03:36:39PM, Eric Blake wrote:
> On 06/25/2010 03:22 PM, Chris Lalancette wrote:
> > There was one major, and a few minor bugs having to do with
> > the reference counting of node devices in daemon/remote.c
> > The major bug was that remoteDispatchNodeDeviceListCaps()
> > was completely failing to unreference node devices; this
> > would lead to many open file descriptors, which would eventually
> > fail.
> > 
> > The minor bugs were along the same lines, but were in rarely
> > used error paths.  Still, they should be corrected.
> > 
> > Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
> > ---
> >  daemon/remote.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/daemon/remote.c b/daemon/remote.c
> > index 91faa9a..ec5f85b 100644
> > --- a/daemon/remote.c
> > +++ b/daemon/remote.c
> > @@ -5012,11 +5012,13 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server *server ATTRIBUTE_UNUSED,
> >          /* remoteDispatchClientRequest will free this. */
> >          char **parent_p;
> >          if (VIR_ALLOC(parent_p) < 0) {
> > +            virNodeDeviceFree(dev);
> >              remoteDispatchOOMError(rerr);
> >              return -1;
> >          }
> 
> ACK.  But given the number of places where you had to duplicate this
> line insertion, would it be better to change all the return -1 into goto
> cleanup, where the cleanup: label guarantees this cleanup for all
> possible exit paths?

Yeah, I considered it.  I'm going to push this patch as-is since it's pretty
important, and then I'll do a follow-up with the cleanup paths.

--
Chris Lalancette

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[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]