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

Re: [libvirt] [PATCH V1 2/4] Implement a virIntSet class



On 01/29/2013 09:52 AM, Stefan Berger wrote:
> Implementation of a virIntSet class.
> 
> ---
>  src/Makefile.am          |    1 
>  src/libvirt_private.syms |    8 +++++
>  src/util/virintset.c     |   74 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virintset.h     |   44 +++++++++++++++++++++++++++
>  4 files changed, 127 insertions(+)

> +
> +ssize_t virIntSetAdd(virIntSetPtr set, int val)

Documentation of these functions would be helpful.

> +{
> +    size_t res;
> +
> +    if (virIntSetIndexOf(set, val) > 0)
> +        return -EEXIST;
> +
> +    if (set->capacity <= set->size) {
> +        if (VIR_REALLOC_N(set->set, set->capacity + 10) < 0) {
> +            return -ENOMEM;
> +        }
> +        set->capacity += 10;
> +    }

Rather than using VIR_REALLOC_N to manage the set size by hand, I
recommend using VIR_RESIZE_N(set->set, set->capacity, set->size, 1).

> +
> +    res = set->size;
> +
> +    set->set[set->size++] = val;
> +
> +    return res;
> +}

Do we ever expect the sets to grow large enough that storing the ints in
sorted order would be beneficial (giving us O(logN) instead of O(N)
behaviors on lookup)?

> +
> +#ifndef __VIR_INTSET_H__
> +# define __VIR_INTSET_H__
> +
> +# include "internal.h"
> +
> +typedef struct _virIntSet virIntSet;
> +typedef virIntSet *virIntSetPtr;
> +
> +struct _virIntSet {
> +    size_t size;
> +    size_t capacity;
> +    int *set;
> +};

This struct could be opaque (moving it into the .c file shouldn't
invalidate any callers, which should all be going through the functions
rather than peeking at the members of the struct).

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

Attachment: signature.asc
Description: OpenPGP digital signature


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