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

Re: [libvirt] [PATCH] Yet more coverity fixes



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


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