From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MW5gJ-0007jR-AC for qemu-devel@nongnu.org; Wed, 29 Jul 2009 05:42:43 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MW5gE-0007hO-AN for qemu-devel@nongnu.org; Wed, 29 Jul 2009 05:42:42 -0400 Received: from [199.232.76.173] (port=47081 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MW5gE-0007hJ-2i for qemu-devel@nongnu.org; Wed, 29 Jul 2009 05:42:38 -0400 Received: from mx2.redhat.com ([66.187.237.31]:37971) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MW5gD-0006HB-JT for qemu-devel@nongnu.org; Wed, 29 Jul 2009 05:42:37 -0400 Message-ID: <4A701AA3.4060902@redhat.com> Date: Wed, 29 Jul 2009 12:47:15 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1248818713-11261-1-git-send-email-lcapitulino@redhat.com> <1248818713-11261-2-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1248818713-11261-2-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: Luiz Capitulino Cc: jan.kiszka@siemens.com, aliguori@us.ibm.com, dlaor@redhat.com, qemu-devel@nongnu.org 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