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