On Mon, Apr 30, 2012 at 12:42:37PM -0400, Stefan Berger wrote:
On 04/30/2012 12:34 PM, Daniel P. Berrange wrote:
On Mon, Apr 30, 2012 at 12:28:26PM -0400, Stefan Berger wrote:
Addressing the following reports:
None of those reports mention daemon/libvirtd.c and yet
These must have gotten lost:
Error: RESOURCE_LEAK: /libvirt/daemon/libvirtd.c:147: open_fn: Calling opening function "open". /libvirt/daemon/libvirtd.c:147: var_assign: Assigning: "stdinfd" = handle returned from "open("/dev/null", 0)". /libvirt/daemon/libvirtd.c:151: noescape: Variable "stdinfd" is not closed or saved in function "dup2". /libvirt/daemon/libvirtd.c:168: leaked_handle: Handle variable "stdinfd" going out of scope leaks the handle.
Error: RESOURCE_LEAK: /libvirt/daemon/libvirtd.c:149: open_fn: Calling opening function "open". /libvirt/daemon/libvirtd.c:149: var_assign: Assigning: "stdoutfd" = handle returned from "open("/dev/null", 1)". /libvirt/daemon/libvirtd.c:153: noescape: Variable "stdoutfd" is not closed or saved in function "dup2". /libvirt/daemon/libvirtd.c:155: noescape: Variable "stdoutfd" is not closed or saved in function "dup2". /libvirt/daemon/libvirtd.c:168: leaked_handle: Handle variable "stdoutfd" going out of scope leaks the handle.
daemon/libvirtd.c | 24 +++++++++++++++++++----- @@ -172,6 +173,19 @@ static int daemonForkIntoBackground(cons _exit(EXIT_SUCCESS); }
+ + cleanup_close_stderr_fileno: + tmpfd = STDERR_FILENO; + VIR_FORCE_CLOSE(tmpfd); + + cleanup_close_stdout_fileno: + tmpfd = STDOUT_FILENO; + VIR_FORCE_CLOSE(tmpfd); + + cleanup_close_stdin_fileno: + tmpfd = STDIN_FILENO; + VIR_FORCE_CLOSE(tmpfd); + cleanup: VIR_FORCE_CLOSE(stdoutfd); VIR_FORCE_CLOSE(stdinfd); This really seems like overkill& ugly. There is no real world leak here since this process will immediately exit.
Right, though this should make coverity quiet...
Really ? I'm not convinced.
/libvirt/daemon/libvirtd.c:168: leaked_handle: Handle variable "stdinfd" going out of scope leaks the handle.
This is refering to the 3rd line here: switch (nextpid) { case 0: /* grandchild */ return statuspipe[1]; Neither the original 'cleanup:' label, nor any of the 3 you added will ever execute in the codepath that coverity is complaining about. 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 :|