[libvirt] [PATCH] Yet more coverity fixes

Daniel P. Berrange berrange at redhat.com
Mon Apr 30 16:34:10 UTC 2012


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

>  daemon/libvirtd.c                         |   24 +++++++++++++++++++-----

> Index: libvirt-acl/daemon/libvirtd.c
> ===================================================================
> --- libvirt-acl.orig/daemon/libvirtd.c
> +++ libvirt-acl/daemon/libvirtd.c
> @@ -141,6 +141,7 @@ static int daemonForkIntoBackground(cons
>              int stdinfd = -1;
>              int stdoutfd = -1;
>              int nextpid;
> +            int tmpfd;
> 
>              VIR_FORCE_CLOSE(statuspipe[0]);
> 
> @@ -151,16 +152,16 @@ static int daemonForkIntoBackground(cons
>              if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO)
>                  goto cleanup;
>              if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO)
> -                goto cleanup;
> +                goto cleanup_close_stdin_fileno;
>              if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO)
> -                goto cleanup;
> +                goto cleanup_close_stdout_fileno;
>              if (stdinfd > STDERR_FILENO && VIR_CLOSE(stdinfd) < 0)
> -                goto cleanup;
> +                goto cleanup_close_stderr_fileno;
>              if (stdoutfd > STDERR_FILENO && VIR_CLOSE(stdoutfd) < 0)
> -                goto cleanup;
> +                goto cleanup_close_stderr_fileno;
> 
>              if (setsid() < 0)
> -                goto cleanup;
> +                goto cleanup_close_stderr_fileno;
> 
>              nextpid = fork();
>              switch (nextpid) {
> @@ -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.

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




More information about the libvir-list mailing list