Re: [PATCH 01/14] phyp: Resolve some file descriptor leaks

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

 



On Wed, Jan 09, 2013 at 07:08:28PM -0700, Eric Blake wrote:
> On 01/09/2013 07:54 AM, John Ferlan wrote:
> > The phypUUIDTable_Push and phypUUIDTable_Pull leaked their fd's on normal
> > return.  I also noted that the Read function had a cut-n-paste error from
> > the write function on a couple of VIR_WARN's
> > 
> > The openSSHSession leaked the sock on the failure path.  Additionally that
> > turns into the internal_socket in the phypOpen code.  That was neither saved
> > nor closed on any path. So I used the connnection_data->sock field to save
> > the socket for eventual close. Of interest here is that phypExec used the
> > connection_data->sock field even though it had never been initialized.
> > ---
> >  src/phyp/phyp_driver.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> > index f89871f..b74cab8 100644
> > --- a/src/phyp/phyp_driver.c
> > +++ b/src/phyp/phyp_driver.c
> > @@ -577,6 +577,7 @@ phypUUIDTable_Push(virConnectPtr conn)
> >          libssh2_channel_free(channel);
> >          channel = NULL;
> >      }
> > +    VIR_FORCE_FCLOSE(fd);
> 
> Who named their FILE* 'fd' anyways?  But not your fault that you are
> trying to clean up some notoriously hairy code.

For anyone feeling brave, with access to suitable hardware, it would
be desirable to port this code over to use virNetSocket which now has
built-in support for libssh2 based tunnelling.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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