Re: [libvirt] [PATCH 4/5] buf: implement generic virBufferEscape

[Eric Blake <eblake@redhat.com>]

On 10/11/2011 04:39 AM, Daniel P. Berrange wrote:
> On Mon, Sep 19, 2011 at 09:13:42PM -0700, Sage Weil wrote:
>> Implement a generic helper to escape a given set of characters with a
>> leading '\'.  Generalizes virBufferEscapeSexpr().
>>
>> Signed-off-by: Sage Weil<sage@newdream.net>
>> @@ -408,14 +428,13 @@ virBufferEscapeSexpr(const virBufferPtr buf,
>>       cur = str;
>>       out = escaped;
>>       while (*cur != 0) {
>> -        switch (*cur) {
>> -        case '\\':
>> -        case '\'':
>> -            *out++ = '\\';
>> -            /* fallthrough */
>> -        default:
>> -            *out++ = *cur;
>> +        for (p = toescape; *p; ++p) {
>> +            if (*cur == *p) {

strchr is slightly more efficient than hand-rolling this loop.

More importantly, you had a logic bug, where you replaced one hard-coded 
instance of "\\'", but not the other (the strcspn optimization).  Also, 
I tend to use "'" instead of "\'", although both work, since I favor 
concise code (I save my ramblings for my emails :).

>
> ACK, trivial isolated patch

I added you to AUTHORS, squashed this in, then pushed.

diff --git i/src/util/buf.c w/src/util/buf.c
index 7d0d2d3..5627d8f 100644
--- i/src/util/buf.c
+++ w/src/util/buf.c
@@ -383,7 +383,7 @@ virBufferEscapeSexpr(const virBufferPtr buf,
                       const char *format,
                       const char *str)
  {
-    virBufferEscape(buf, "\\\'", format, str);
+    virBufferEscape(buf, "\\'", format, str);
  }

  /**
@@ -414,7 +414,7 @@ virBufferEscape(const virBufferPtr buf,
          return;

      len = strlen(str);
-    if (strcspn(str, "\\'") == len) {
+    if (strcspn(str, toescape) == len) {
          virBufferAsprintf(buf, format, str);
          return;
      }
@@ -428,12 +428,8 @@ virBufferEscape(const virBufferPtr buf,
      cur = str;
      out = escaped;
      while (*cur != 0) {
-        for (p = toescape; *p; ++p) {
-            if (*cur == *p) {
-                *out++ = '\\';
-                break;
-            }
-        }
+        if (strchr(toescape, *cur))
+            *out++ = '\\';
          *out++ = *cur;
          cur++;
      }
-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org