[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH v2 08/17] virstring: Introduce virStringIsNull()



On Thu, Aug 20, 2020 at 01:02:55PM +0200, Peter Krempa wrote:
> On Tue, Jul 07, 2020 at 21:46:26 +0200, Michal Privoznik wrote:
> > This function will be used to detect zero buffers (which are
> > going to be interpreted as hole in virStream later).
> > 
> > I shamelessly took inspiration from coreutils.
> 
> Coreutils is proudly GPLv3 ...
> 
> > Signed-off-by: Michal Privoznik <mprivozn redhat com>
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/virstring.c     | 38 ++++++++++++++++++++++++++++++++
> >  src/util/virstring.h     |  2 ++
> >  tests/virstringtest.c    | 47 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 88 insertions(+)
> 
> [...]
> 
> > diff --git a/src/util/virstring.c b/src/util/virstring.c
> > index e9e792f3bf..c26bc770d4 100644
> > --- a/src/util/virstring.c
> > +++ b/src/util/virstring.c
> > @@ -1404,3 +1404,41 @@ int virStringParseYesNo(const char *str, bool *result)
> >  
> >      return 0;
> >  }
> > +
> > +
> > +/**
> > + * virStringIsNull:
> 
> IsNull might indicate that this does a check if the pointer is NULL. You
> are checking for NUL bytes.
> 
> > + * @buf: buffer to check
> > + * @len: the length of the buffer
> > + *
> > + * For given buffer @buf and its size @len determine whether
> > + * it contains only zero bytes (NUL) or not.
> 
> Given the semantics of C strings being terminated by the NUL byte I
> don't think this function qualifies as a string helper and thus should
> probably reside somewhere outside of virstring.h
> 
> > + *
> > + * Returns: true if buffer is full of zero bytes,
> > + *          false otherwise.
> > + */
> > +bool virStringIsNull(const char *buf, size_t len)
> > +{
> > +    const char *p = buf;
> > +
> > +    if (!len)
> > +        return true;
> > +
> > +    /* Check up to 16 first bytes. */
> > +    for (;;) {
> > +        if (*p)
> > +            return false;
> > +
> > +        p++;
> > +        len--;
> > +
> > +        if (!len)
> > +            return true;
> > +
> > +        if ((len & 0xf) == 0)
> > +            break;
> > +    }
> 
> Do we really need to do this optimization? We could arguably simplify
> this to:
> 
>     if (*buf != '\0')
>         return false;
> 
>     return memcmp(buf, buf + 1, len - 1);

Depends whether we care about this sparsification having goood
performance or not. As a point of reference, QEMU has invested
tonnes of effort in its impl, using highly tuned impls for
SSE, AVX, etc switched at runtime.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]