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