Re: [libvirt] [PATCH 3/3] Use virBufferEscapeShell in virNetSocketNewConnectSSH

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 eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org