From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:43480)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RWY8i-0006xI-5w
	for qemu-devel@nongnu.org; Fri, 02 Dec 2011 13:47:17 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RWY8h-0006FG-3g
	for qemu-devel@nongnu.org; Fri, 02 Dec 2011 13:47:16 -0500
Received: from mail-gy0-f173.google.com ([209.85.160.173]:34213)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RWY8g-0006FA-Ti
	for qemu-devel@nongnu.org; Fri, 02 Dec 2011 13:47:15 -0500
Received: by ghbz10 with SMTP id z10so238997ghb.4
	for <qemu-devel@nongnu.org>; Fri, 02 Dec 2011 10:47:14 -0800 (PST)
Message-ID: <4ED91D2E.4050308@codemonkey.ws>
Date: Fri, 02 Dec 2011 12:47:10 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
References: <1322687028-29714-1-git-send-email-aliguori@us.ibm.com>	<1322687028-29714-2-git-send-email-aliguori@us.ibm.com>
	<4ED7A2A1.80002@redhat.com>
In-Reply-To: <4ED7A2A1.80002@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 01/18] qom: add new dynamic property
 infrastructure based on Visitors
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>, Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>, Jan Kiszka <jan.kiszka@siemens.com>

On 12/01/2011 09:52 AM, Kevin Wolf wrote:
> Am 30.11.2011 22:03, schrieb Anthony Liguori:
>> +
>> +void qdev_property_add(DeviceState *dev, const char *name, const char *type,
>> +                       DevicePropertyEtter *get, DevicePropertyEtter *set,
>> +                       DevicePropertyRelease *release, void *opaque,
>> +                       Error **errp)
>
> How about letting the caller pass in a DeviceProperty for improved
> readability and usability? Instead of memorizing the order of currently
> eight parameters (could probably become more in the future) you can use
> proper C99 initializers then.

This ends up making the code much more complex for the client if you try to 
eliminate the opaque and replace it with the structure.  It becomes necessary to 
do a dynamic allocation of the structure and then you also have to add a release 
function.

We could make the structure just contain the function pointers and not the 
opaque but that doesn't seem very helpful to me.  It just adds a few extra lines 
to the client code without a lot of gain.

Regards,

Anthony Liguori

>
>> @@ -45,6 +82,7 @@ struct DeviceState {
>>       QTAILQ_ENTRY(DeviceState) sibling;
>>       int instance_id_alias;
>>       int alias_required_for_version;
>> +    GSList *properties;
>>   };
>
> Why GSList instead of qemu-queue.h macros that would provide type safety?
>
> I don't think a property can belong to multiple devices, can it?
> qdev_property_add only refers to a single device, and nothing else adds
> elements to the list.
>
> Kevin
>