On Mon, Aug 01, 2011 at 02:28:59PM -0600, Eric Blake wrote:
On 07/29/2011 03:12 AM, Guido Günther wrote:
>Quote strings so they're safe to pass to the shell. Based on glib's
>g_quote_string.
>---
> src/libvirt_private.syms | 1 +
> src/util/buf.c | 29 +++++++++++++++++++++++++++++
> src/util/buf.h | 1 +
> 3 files changed, 31 insertions(+), 0 deletions(-)
>
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>index 853ee62..eb16fb6 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -28,6 +28,7 @@ virBufferError;
> virBufferEscapeSexpr;
> virBufferEscapeString;
> virBufferFreeAndReset;
>+virBufferQuoteString;
I'd prefer that we had the term Shell somewhere in the name, and
perhaps keep it similar to other escaping functions. How about:
virBufferEscapeShell;
Also, virsh.c could use this new function in its implementation of
'virsh echo --shell'.
> /**
>+ * virBufferQuoteString:
>+ * @buf: the buffer to append to
>+ * @str: an unquoted string
>+ *
>+ * Quotes a string so that the shell (/bin/sh) will interpret the
>+ * quoted string as unquoted_string.
>+ */
>+void
>+virBufferQuoteString(const virBufferPtr buf, const char *unquoted_str)
>+{
>+ const char *p;
>+
>+ virBufferAddChar(buf, '\'');
>+ p = unquoted_str;
I'd _really_ like to make this smart enough to omit outer '' if the
entire string does not need quoting (again, see how virsh cmdEcho
does it).
>+
>+ while (*p) {
>+ /* Replace literal ' with a close ', a \', and a open ' */
>+ if (*p == '\'')
>+ virBufferAddLit(buf, "'\\''");
>+ else
>+ virBufferAddChar(buf, *p);
That's a bit slow compared to the other virBufferEscape* methods
which pre-allocate based on worst case, then do things directly into
the output buffer rather than making a function call for every byte.
Overall, the idea is nice, and I also imagine using it in
virCommandToString to output unambiguous log messages for commands
about to be run, but it's not quite ready yet.
I'm torn on whether to touch this up and include it in 0.9.4 (since
it is turning into a frequently reported issue), or defer it to
post-release since it is starting to grow in scope.
IMO we have too many fixes going in to 0.9.4 at the last minute and I
think we ought to wait to 0.9.5 on this one.
Dave