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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 29 Jul 2009 12:47:15 +0300
Avi Kivity <avi@redhat.com> 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.

 Will change.

> > +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.

 I was thinking in doing a little bit different.

 My next patchset (phase 2) will introduce the QType (or QObject)
data type, as you have suggested in the QMP thread. This one will
have all those methods to convert from int, string, dict etc.

 Then the dictionary can store it and the user can provide
a iterator to print the objects.

 So, the point here is where to have the high-level data type
conversion: in the dict itself or in a higher layer (QObject).

 I slightly prefer to have them in the QObject, this way the
dict is more flexible and simpler, capable of storing anything.

 But I don't known where the clone() method should be, maybe in
both?

PS: I've called the iterator walk(), weird.