On 10/12/2011 04:40 PM, Guido Günther wrote:
to escape the netcat command since it's passed to the shell. Adjust
expected test case output accordingly.
---
  src/rpc/virnetsocket.c   |   25 ++++++++++++++++++++-----
  tests/virnetsockettest.c |   10 +++++-----
  2 files changed, 25 insertions(+), 10 deletions(-)

ACK with nits fixed. Still to go - it would be nice to follow up with a 4/3 that converts virsh.c cmdEcho to use virBufferEscapeShell.

@@ -636,6 +639,19 @@ int virNetSocketNewConnectSSH(const char *nodename,
          virCommandAddArgList(cmd, "-o", "StrictHostKeyChecking=no", NULL);

      virCommandAddArgList(cmd, nodename, "sh", "-c", NULL);
+
+    virBufferEscapeShell(&buf, netcat ? netcat : "nc");
+    if (virBufferError(&buf)) {
+        virBufferFreeAndReset(&buf);
+        virReportOOMError();
+        return -1;
+    }
+    quoted = virBufferContentAndReset(&buf);
+    if (quoted == NULL) {

You are guaranteed that quoted is not NULL, by virtue of the fact that you already filtered for virBufferError just beforehand. Drop this if statement.

+++ b/tests/virnetsockettest.c
@@ -496,7 +496,7 @@ mymain(void)
      struct testSSHData sshData1 = {
          .nodename = "somehost",
          .path = "/tmp/socket",
-        .expectOut = "somehost sh -c 'if nc -q 2>&1 | grep \"requires an 
argument\">/dev/null 2>&1; then     ARG=-q0;fi;nc $ARG -U /tmp/socket'\n",
+        .expectOut = "somehost sh -c 'if 'nc' -q 2>&1 | grep \"requires an 
argument\">/dev/null 2>&1; then     ARG=-q0;fi;'nc' $ARG -U /tmp/socket'\n",
      };

Rebase this on top of my comments for 1/3 (no long lines). Also, add one more test where $NC is something that actually proves things work with shell meta-characters, perhaps by passing 'nc -4' as the value that eventually supplies the %s for $NC, so that .expectOut will include: sh -c 'if ''nc -4'' -q 2>&1 ....

--
Eric Blake   ebl...@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Reply via email to