Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
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.
+ *
+ * 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;
+ }
+ }
+
+ 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);
+}
+
+
/**
* virBufferURIEncodeString:
* @buf: the buffer to append to
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index 144a1ba06e..94f14b5b16 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -82,6 +82,8 @@ void virBufferStrcat(virBufferPtr buf, ...)
ATTRIBUTE_SENTINEL;
void virBufferEscape(virBufferPtr buf, char escape, const char *toescape,
const char *format, const char *str);
+void virBufferEscapeN(virBufferPtr buf, const char *format,
+ const char *str, ...);
void virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str);
void virBufferEscapeSexpr(virBufferPtr buf, const char *format,
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index 22407ab6a8..34160e6b28 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -376,6 +376,35 @@ testBufEscapeStr(const void *opaque ATTRIBUTE_UNUSED)
static int
+testBufEscapeN(const void *opaque)
+{
+ const struct testBufAddStrData *data = opaque;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *actual;
+ int ret = -1;
+
+ virBufferEscapeN(&buf, "%s", data->data, '\\', "=", ',', ",", NULL);
+
+ if (!(actual = virBufferContentAndReset(&buf))) {
+ VIR_TEST_DEBUG("testBufEscapeN: buf is empty");
+ goto cleanup;
+ }
+
+ if (STRNEQ_NULLABLE(actual, data->expect)) {
+ VIR_TEST_DEBUG("testBufEscapeN: Strings don't match:\n");
+ virTestDifference(stderr, data->expect, actual);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(actual);
+ return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -422,6 +451,18 @@ mymain(void)
DO_TEST_ESCAPE("\x01\x01\x02\x03\x05\x08",
"<c>\n <el></el>\n</c>");
+#define DO_TEST_ESCAPEN(data, expect) \
+ do { \
+ struct testBufAddStrData info = { data, expect }; \
+ if (virTestRun("Buf: EscapeN", testBufEscapeN, &info) < 0) \
+ ret = -1; \
+ } while (0)
+
+ DO_TEST_ESCAPEN("noescape", "noescape");
+ DO_TEST_ESCAPEN("comma,escape", "comma,,escape");
+ DO_TEST_ESCAPEN("equal=escape", "equal\\=escape");
+ DO_TEST_ESCAPEN("comma,equal=escape", "comma,,equal\\=escape");
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.11.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Feb 23, 2017 at 04:26:58PM +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> 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);
> + *
> + * 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
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 <phrdina@redhat.com>
> > ---
> > 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
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Feb 23, 2017 at 05:51:21PM +0100, Pavel Hrdina wrote:
> 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 <phrdina@redhat.com>
> > > ---
> > > 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.
Oh, that's the opposite way around to what I thought you were
doing ! I thought a single character could be escaped to a string
of multiple characters, but you have a string containing a set
of characters that must be escaped to a single character.
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
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.