Re: [PATCH v2 08/17] virstring: Introduce virStringIsNull()
On 8/20/20 1:02 PM, 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 ...
Sure. But it was discussed in v1 and I think we agreed that the
algorithm is generic enough that it can be used in Libvirt too:
https://www.redhat.com/archives/libvir-list/2020-July/msg00170.html
> Signed-off-by: Michal Privoznik <mprivozn(a)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.
Fair enough. I'm out of ideas though. Do you have a suggestion?
> + * @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
Alright. I will try to find better location. But since I want to use
this function from virsh too I'd like to have it in utils.
> + *
> + * 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.
The idea is to pass aligned address to memcmp(). But I guess we can let
memcmp() deal with that.
Michal