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