[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] Add virBufferQuoteString



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.

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]