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

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



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);

You can then use the saved lines to explain that comparing a piece of
memory with itself shifted by any position just ensures that there are
repeating sequences of itself in the remainder and by shifting it by 1
it means that it checks that the strings are just the same byte. The
check above then ensuers that the one byte is NUL.


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