[libvirt] [PATCH] tests: Fix VIR_TEST_REGENERATE_OUTPUT

Laine Stump laine at laine.org
Wed Apr 13 00:54:59 UTC 2016


On 04/12/2016 06:33 PM, Cole Robinson wrote:
> commit 950a90d489 mocked some virCommand handling for the qemu tests,
> but we were using that in the test suite to call test-wrap-argv.pl for
> regenerating test output.
>
> Switch the generator code to just use system() instead
> ---

I dislike giving up the potential utility of virCommand*() in tests, but 
this does fix the problem (without creating the inefficiency of an extra 
function in the callstack for every virCommandRun() call by every 
libvirtd in the world). So ACK unless someone has a better idea / 
different opinion. (But there's one thing to fix, noted below).

(if this was in a binary run outside the build environment, I would NACK 
use of system(), but since it's only used by a test...)


>   tests/testutils.c | 19 +++++--------------
>   1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/tests/testutils.c b/tests/testutils.c
> index a0ce4b6..4f3e67b 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -434,24 +434,15 @@ virTestRewrapFile(const char *filename)
>   {
>       int ret = -1;
>       char *outbuf = NULL;

outbuf is no longer used, so it should be removed.

> -    char *script = NULL;
> -    virCommandPtr cmd = NULL;
> +    char *cmd = NULL;
>   
> -    if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
> +    if (virAsprintf(&cmd, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s",
> +                    abs_srcdir, filename, filename) < 0)
>           goto cleanup;

For that matter, cmd would be NULL if virAsprintf failed, so all that's 
really needed here is a "return -1;", meaning that the cleanup label can 
also be removed.

>   
> -    cmd = virCommandNewArgList(script, filename, NULL);
> -    virCommandSetOutputBuffer(cmd, &outbuf);
> -    if (virCommandRun(cmd, NULL) < 0)
> -        goto cleanup;
> -
> -    if (virFileWriteStr(filename, outbuf, 0666) < 0)
> -        goto cleanup;
> -
> -    ret = 0;
> +    ret = system(cmd);
>    cleanup:
> -    VIR_FREE(script);
> -    virCommandFree(cmd);
> +    VIR_FREE(cmd);
>       VIR_FREE(outbuf);
>       return ret;
>   }




More information about the libvir-list mailing list