Re: [Qemu-devel] [PATCHv2 2/9] cutils: add a function to find non-zero content in a buffer

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On 03/15/2013 09:50 AM, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/qemu-common.h |    2 ++
>  util/cutils.c         |   40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 

>  
> +#define BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR 8

Good.

> +    assert(len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR 
> +        * sizeof(VECTYPE)) == 0);

Good use of it.

> +    assert(((uintptr_t) buf) % sizeof(VECTYPE) == 0);
> +
> +    if (*((const long *) buf)) {
> +        return 0;
> +    }
> +
> +    for (i = 0; i < len / sizeof(VECTYPE); i += 8) {

Magic number 8, instead of reusing BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR.

> +		VECTYPE tmp0 = p[i+0] | p[i+1];

Indentation looks off, because you used TABs.

Spaces around binary operators (two instances of '+' per line).

> +		VECTYPE tmp1 = p[i+2] | p[i+3];
> +		VECTYPE tmp2 = p[i+4] | p[i+5];
> +		VECTYPE tmp3 = p[i+6] | p[i+7];
> +		VECTYPE tmp01 = tmp0 | tmp1;
> +		VECTYPE tmp23 = tmp2 | tmp3;
> +		if (!ALL_EQ(tmp01 | tmp23, zero)) {
> +		    break;
> +		}
> +    }
> +    return i * sizeof(VECTYPE);
> +}

Algorithm looks correct, but worth a respin to fix the appearance.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]