[libvirt] [PATCH 2/5] commandtest: Resolve some coverity resource leaks

[Peter Krempa]

On 02/14/13 17:42, John Ferlan wrote:
> ---
>   tests/commandtest.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tests/commandtest.c b/tests/commandtest.c
> index 93c6333..3bfc358 100644
> --- a/tests/commandtest.c
> +++ b/tests/commandtest.c

...

> @@ -963,16 +964,19 @@ mymain(void)
>           dup2(fd, 6) < 0 ||
>           dup2(fd, 7) < 0 ||
>           dup2(fd, 8) < 0 ||
> -        (fd > 8 && VIR_CLOSE(fd) < 0))
> +        (fd > 8 && VIR_CLOSE(fd) < 0)) {
> +        VIR_FORCE_CLOSE(fd);
>           return EXIT_FAILURE;
> +    }
> +    sa_assert(fd == -1);

according to man of dup2():
*  If  oldfd  is a valid file descriptor, and newfd has the same value 
as oldfd, then dup2() does nothing, and returns newfd.

It is possible that open returns fd < 8, dup2() on that does nothing and 
afterwards this assertion won't be true.

>
>       /* Prime the debug/verbose settings from the env vars,
>        * since we're about to reset 'environ' */
>       ignore_value(virTestGetDebug());
>       ignore_value(virTestGetVerbose());
>
> -    if (virInitialize() < 0)
> -        return EXIT_FAILURE;
> +    /* Make sure to not leak fd's */
> +    virinitret = virInitialize();
>
>       /* Phase two of killing interfering fds; see above.  */
>       fd = 3;

ACK with the assertion removed or a sufficient explanation provided.

Peter