[libvirt] [PATCH 3/4] rpc: avoid memory leak on error

[Daniel P. Berrange]

On Fri, Jul 01, 2011 at 04:54:12PM -0600, Eric Blake wrote:
> On 06/30/2011 07:05 PM, Wen Congyang wrote:
> > At 07/01/2011 07:36 AM, Eric Blake Write:
> >> Detected by Coverity.  The leak is on an error path, but I'm not
> >> sure whether that path is likely to be triggered in practice.
> >>
> 
> >>  error:
> >>      virNetSocketFree(clientsock);
> >> +    virNetServerClientFree(client);
> > 
> > If svc->dispatchFunc is NULL, we will goto here to do cleanup. Unfortunately,
> > client->sock is clientsock and it will be freed again in virNetServerClientFree().
> > It may cause libvirtd crashed.
> > 
> > If svc->dispatchFunc() failed, we close and free client. But we only free client here,
> > not close it here. Do we need to close it here?
> > 
> > I think the cleanup code should like this:
> > 
> > if (client) {
> >     virNetServerClientClose(client);
> >     virNetServerClientFree(client);
> > } else {
> >     virNetSocketFree(clientsock);
> > }
> 
> Makes sense to me, but I'd rather hear Dan's take on it as author of
> that code.

Yep, ACK to Wen's version of the code

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