From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MW9Tz-0000lv-Ik for qemu-devel@nongnu.org; Wed, 29 Jul 2009 09:46:15 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MW9Tu-0000gl-7W for qemu-devel@nongnu.org; Wed, 29 Jul 2009 09:46:15 -0400 Received: from [199.232.76.173] (port=55990 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MW9Tt-0000gh-TF for qemu-devel@nongnu.org; Wed, 29 Jul 2009 09:46:09 -0400 Received: from mx2.redhat.com ([66.187.237.31]:54157) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MW9Tt-0002pg-Ap for qemu-devel@nongnu.org; Wed, 29 Jul 2009 09:46:09 -0400 Date: Wed, 29 Jul 2009 10:46:00 -0300 From: Luiz Capitulino Message-ID: <20090729104600.17deb355@doriath> In-Reply-To: <4A701AA3.4060902@redhat.com> References: <1248818713-11261-1-git-send-email-lcapitulino@redhat.com> <1248818713-11261-2-git-send-email-lcapitulino@redhat.com> <4A701AA3.4060902@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 01/25] Introduce QEMU dictionary data type List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: jan.kiszka@siemens.com, aliguori@us.ibm.com, dlaor@redhat.com, qemu-devel@nongnu.org On Wed, 29 Jul 2009 12:47:15 +0300 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. 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.