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

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

 



On 02/14/2013 09:42 AM, 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
> @@ -680,7 +680,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED)
>          goto cleanup;
>      }
>  
> -    if (!outbuf || *outbuf) {
> +    if (*outbuf) {
>          puts("output buffer is not an allocated empty string");
>          goto cleanup;
>      }
> @@ -702,7 +702,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED)
>          goto cleanup;
>      }
>  
> -    if (!outbuf || *outbuf || !errbuf || *errbuf) {
> +    if (*outbuf || *errbuf) {
>          puts("output buffers are not allocated empty strings");
>          goto cleanup;

This part seems reasonable.

>      }
> @@ -936,6 +936,7 @@ mymain(void)
>      int fd;
>      virCommandTestDataPtr test = NULL;
>      int timer = -1;
> +    int virinitret;
>  
>      if (virThreadInitialize() < 0)
>          return EXIT_FAILURE;
> @@ -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;

Odd that Coverity complains about the leak of fd, but not about the leak
of all the other dup2'd fds; and at any rate, the leak is
inconsequential since we are about to exit.  But if this shuts things
up, I'm okay with it.

> +    }
> +    sa_assert(fd == -1);

Yes, this is true, and if it helps Coverity, I'm fine with it.

>  
>      /* 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;
> @@ -988,6 +992,9 @@ mymain(void)
>      fd = 8;
>      VIR_FORCE_CLOSE(fd);
>  
> +    if (virinitret < 0)
> +        return EXIT_FAILURE;

Seems okay to me.

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]