Re: [PATCHv2 0/4] Add LibSSH2 transport

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

 



On Mon, Aug 13, 2012 at 10:12:01AM -0500, Doug Goldstein wrote:
> On Mon, Aug 13, 2012 at 4:53 AM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote:
> > On Sun, Aug 12, 2012 at 03:30:16PM -0500, Doug Goldstein wrote:
> >> On Sat, Aug 11, 2012 at 4:20 PM, Peter Krempa <pkrempa@xxxxxxxxxx> wrote:
> >> > This is a second respin of the LibSSH2 transport patch series.
> >>
> >> Annoyingly Gmail has decided to eat patch 1/4 so I've been reviewing
> >> it on the ML archives. I've noticed the following:
> >>
> >> virNetLibSSH2CheckHostKey() the virBuffer buff is leaked in a few
> >> places. After the following line:
> >>
> >> keyhash = virBufferContentAndReset(&buff);
> >>
> >> buff isn't freed in all the cases that you can return.
> >
> > Actually its ok. The virBuffer instance for 'buff' is stack allocated.
> > The only dynamically allocated memory is the string inside the buf which
> > is what virBufferContentAndReset is giving you. So 'keyhash' is what
> > needs to be free'd & it looks to be.
> >
> > I thought we had this documented on the website somewhere, but I guess
> > not. Here's the original API description for reference:
> >
> >   https://www.redhat.com/archives/libvir-list/2008-April/msg00371.html
> >
> > Daniel
> > --
> 
> My mistake. Sorry for the extra noise Peter.

No problem, we appreciate anyone who takes the time todo code reviews :-)


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]