diff mbox

tests: Use system() for test-wrap-argv.pl

Message ID 6f895b748c76b799dd4f2b0f4b96fd20562a5310.1461173530.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson April 20, 2016, 5:33 p.m. UTC 
virCommand doesn't buy us anything here, and this simplifies the
code. There's another usage of system() in testutils.c already FWIW,
though it predates virCommand
---
Had this sitting in a branch from an earlier discussion... I figure
system() is safe in the test suite but I'm not going to argue if
someone feels strongly the other way

 tests/testutils.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

-- 
2.7.3

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

Comments

Cole Robinson April 21, 2016, 1:33 p.m. UTC | #1 
On 04/21/2016 04:04 AM, Sascha Silbe wrote:
> Dear Cole,

> 

> Cole Robinson <crobinso@redhat.com> writes:

> 

> [tests/testutils.c:virTestRewrapFile()]

>> +    /* The 'echo' syntax lets us read and write the file in one shot */

>> +    if (virAsprintf(&cmd, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s",

>> +                    abs_srcdir, filename, filename) < 0)

>> +        return -1;

> [...]

>> +    return system(cmd);

> 

> Doesn't the shell truncate the file before test-wrap-argv.pl gets a

> chance to read the original content?

> 


That's what the echo syntax works around; the test-wrap-argv command is fully
run before the output file is truncated

But I've dropped this patch anyways

- Cole

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

Patch

diff --git a/tests/testutils.c b/tests/testutils.c
index 79d0763..7664b06 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -432,28 +432,14 @@  virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED,
 static int
 virTestRewrapFile(const char *filename)
 {
-    int ret = -1;
-    char *outbuf = NULL;
-    char *script = NULL;
-    virCommandPtr cmd = NULL;
-
-    if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
-        goto cleanup;
-
-    cmd = virCommandNewArgList(script, filename, NULL);
-    virCommandSetOutputBuffer(cmd, &outbuf);
-    if (virCommandRun(cmd, NULL) < 0)
-        goto cleanup;
+    char *cmd;
 
-    if (virFileWriteStr(filename, outbuf, 0666) < 0)
-        goto cleanup;
+    /* The 'echo' syntax lets us read and write the file in one shot */
+    if (virAsprintf(&cmd, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s",
+                    abs_srcdir, filename, filename) < 0)
+        return -1;
 
-    ret = 0;
- cleanup:
-    VIR_FREE(script);
-    virCommandFree(cmd);
-    VIR_FREE(outbuf);
-    return ret;
+    return system(cmd);
 }
 
 /**