[Qemu-devel] Re: [PATCH 01/25] Introduce QEMU dictionary data type

[Avi Kivity]

On 07/29/2009 01:04 AM, Luiz Capitulino wrote:
> +struct qemu_dict {
> +    size_t size;
> +    struct qemu_dict_entry *table[QEMU_DICT_HASH_SIZE];
> +};
>    

We don't need to prefix everything with qemu, I think QDict or even Dict 
is sufficient.  Ditto for the function names.  Also please provide a 
typedef as is common qemu practice.


> +
> +struct qemu_dict *qemu_dict_create(void);
> +void qemu_dict_add(struct qemu_dict *qdict, const char *key, void *value);
> +void *qemu_dict_get(const struct qemu_dict *qdict, const char *key);
> +int qemu_dict_exists(const struct qemu_dict *qdict, const char *key);
> +void *qemu_dict_del(struct qemu_dict *qdict, const char *key);
> +void qemu_dict_destroy(struct qemu_dict *qdict);
> +void qemu_dict_walk(const struct qemu_dict *qdict,
> +                    void (*walkf)(const char *key, void *value));
> +
>    

I'm worried about all those void *s as they move responsibility for type 
safety and lifecycle management to the user.  I'd much rather see a 
QObject (or Object) with the following methods:

  clone() - deep copy an object; dicts will store copies so we'll avoid 
those leaks or a dictionary member modified after it was stored
  destroy()
  type() - return the type
  as_dict(), as_string(), as_int() - convert to a subclass so its 
methods can be used

Consider an operation such as printing out the dictionary, you have to 
know the types of the values.

--
error compiling committee.c: too many arguments to function