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

[Anthony Liguori]

Avi Kivity wrote:
> 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()

It probably would be just as easy to do a ref()/unref() such that the 
deep copying was avoided.

--
Regards,

Anthony Liguori