[libvirt] [PATCH 1/2] Add virBufferQuoteString

[Dave Allan]

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