On Thu, Feb 23, 2017 at 04:32:32PM +0000, Daniel P. Berrange wrote: > On Thu, Feb 23, 2017 at 04:26:58PM +0100, Pavel Hrdina wrote: > > Signed-off-by: Pavel Hrdina <[email protected]> > > --- > > src/libvirt_private.syms | 1 + > > src/util/virbuffer.c | 104 > > +++++++++++++++++++++++++++++++++++++++++++++++ > > src/util/virbuffer.h | 2 + > > tests/virbuftest.c | 41 +++++++++++++++++++ > > 4 files changed, 148 insertions(+) > > > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > > index e9c4d73779..6ce32f1101 100644 > > --- a/src/libvirt_private.syms > > +++ b/src/libvirt_private.syms > > @@ -1286,6 +1286,7 @@ virBufferContentAndReset; > > virBufferCurrentContent; > > virBufferError; > > virBufferEscape; > > +virBufferEscapeN; > > virBufferEscapeSexpr; > > virBufferEscapeShell; > > virBufferEscapeString; > > diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c > > index d582e7dbec..ad6b29951e 100644 > > --- a/src/util/virbuffer.c > > +++ b/src/util/virbuffer.c > > @@ -33,6 +33,7 @@ > > #include "virbuffer.h" > > #include "viralloc.h" > > #include "virerror.h" > > +#include "virstring.h" > > > > > > /* If adding more fields, ensure to edit buf.h to match > > @@ -588,6 +589,109 @@ virBufferEscape(virBufferPtr buf, char escape, const > > char *toescape, > > VIR_FREE(escaped); > > } > > > > + > > +struct _virBufferEscapePair { > > + char escape; > > + char *toescape; > > +}; > > + > > + > > +/** > > + * virBufferEscapeN: > > + * @buf: the buffer to append to > > + * @format: a printf like format string but with only one %s parameter > > + * @str: the string argument which needs to be escaped > > + * @...: the variable list of arguments composed > > + * > > + * The variable list of arguments @... must be composed of > > + * 'char escape, char *toescape' pairs followed by NULL. > > So most of the complexity you have here is to deal with the ability > to turn a single character into a string of escaped characters. Unless > I'm missing something, you don't actually use that ability in practice. > If we make it more restrictive so you just have a 1-1 mapping we can > simplify the API & implementation. eg > > virBufferEscapeN(virBufferPtr buf, > const char *format, > const char *str, > const char *forbidden, > const char *replacements) > > with strlen(forbidden) == strlen(replacements);
The idea was that a group of characters can be escaped with one character,
so far the usage seems to be really simple. Currently this would work and
it would be really simple:
virBufferEscapeN(buf, "%s", str, ",=", ",\\");
but imagine this scenario:
virBufferEscapeN(buf, "%s", str, ",=%&*()", ",\\\\\\\\\\\\");
virBufferEscapeN(buf, "%s", str, ',', ",", '\\', "=%&*()", NULL);
I think that the second approach is better and easier for user of
virBufferEscapeN.
> > + *
> > + * This has the same functionality as virBufferEscape with the extension
> > + * that allows to specify multiple pairs of chars that needs to be escaped.
> > + */
> > +void
> > +virBufferEscapeN(virBufferPtr buf,
> > + const char *format,
> > + const char *str,
> > + ...)
> > +{
> > + int len;
> > + size_t i;
> > + char escape;
> > + char *toescape;
> > + char *toescapeall = NULL;
> > + char *escaped = NULL;
> > + char *out;
> > + const char *cur;
> > + struct _virBufferEscapePair escapeItem;
> > + struct _virBufferEscapePair *escapeList = NULL;
> > + size_t nescapeList = 0;
> > + va_list ap;
> > +
> > + if ((format == NULL) || (buf == NULL) || (str == NULL))
> > + return;
> > +
> > + if (buf->error)
> > + return;
> > +
> > + va_start(ap, str);
> > +
> > + while ((escape = va_arg(ap, int))) {
> > + if (!(toescape = va_arg(ap, char *))) {
> > + virBufferSetError(buf, errno);
> > + goto cleanup;
> > + }
> > +
> > + escapeItem.escape = escape;
> > + escapeItem.toescape = toescape;
> > +
> > + if (VIR_STRCAT_QUIET(toescapeall, toescape) < 0) {
> > + virBufferSetError(buf, errno);
> > + goto cleanup;
> > + }
> > +
> > + if (VIR_APPEND_ELEMENT_QUIET(escapeList, nescapeList, escapeItem)
> > < 0) {
> > + virBufferSetError(buf, errno);
> > + goto cleanup;
> > + }
> > + }
>
> This whole loop would go away with the simpler API contract, which
> nicely avoids doing allocations in the loop body.
>
>
> > + len = strlen(str);
> > + if (strcspn(str, toescapeall) == len) {
> > + virBufferAsprintf(buf, format, str);
> > + goto cleanup;
> > + }
> > +
> > + if (xalloc_oversized(2, len) ||
> > + VIR_ALLOC_N_QUIET(escaped, 2 * len + 1) < 0) {
> > + virBufferSetError(buf, errno);
> > + goto cleanup;
> > + }
> > +
> > + cur = str;
> > + out = escaped;
> > + while (*cur != 0) {
> > + for (i = 0; i < nescapeList; i++) {
> > + if (strchr(escapeList[i].toescape, *cur)) {
> > + *out++ = escapeList[i].escape;
> > + break;
> > + }
> > + }
> > + *out++ = *cur;
> > + cur++;
> > + }
> > + *out = 0;
> > +
> > + virBufferAsprintf(buf, format, escaped);
> > +
> > + cleanup:
> > + va_end(ap);
> > + VIR_FREE(toescapeall);
> > + VIR_FREE(escapeList);
> > + VIR_FREE(escaped);
> > +}
>
> Regards,
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
>
> --
> libvir-list mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
