[libvirt] [PATCH 3/5] util: virbuffer: introduce virBufferEscapeN

[Daniel P. Berrange]

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 at 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/ :|