On Thu, Aug 20, 2020 at 14:36:15 +0200, Michal Privoznik wrote: > 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@xxxxxxxxxx> > > > --- > > > 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. Well, this explanation might justify the algorithm above, but it's certainly not obvious, so please add a comment.
