All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
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>
Subject: Re: [Qemu-devel] [PATCH 01/18] qom: add new dynamic property infrastructure based on Visitors
Date: Mon, 05 Dec 2011 10:16:23 +0100	[thread overview]
Message-ID: <4EDC8BE7.1000402@redhat.com> (raw)
In-Reply-To: <4ED91D2E.4050308@codemonkey.ws>

Am 02.12.2011 19:47, schrieb Anthony Liguori:
> 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.

Hm, why doesn't static allocation work with it?

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

I keep switching back and forth between mails to find out what these
parameters are supposed to mean (especially the NULL ones), so yes, I
think even passing just the name and function pointers this way would
improve readability.

Kevin

  reply	other threads:[~2011-12-05  9:13 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-30 21:03 [Qemu-devel] [PATCH 00/18] qom: dynamic properties and composition tree Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 01/18] qom: add new dynamic property infrastructure based on Visitors Anthony Liguori
2011-12-01  8:19   ` Stefan Hajnoczi
2011-12-01 13:30     ` Anthony Liguori
2011-12-01 15:52   ` Kevin Wolf
2011-12-02  1:08     ` Anthony Liguori
2011-12-02  9:43       ` Kevin Wolf
2011-12-02 18:47     ` Anthony Liguori
2011-12-05  9:16       ` Kevin Wolf [this message]
2011-11-30 21:03 ` [Qemu-devel] [PATCH 02/18] qom: register legacy properties as new style properties Anthony Liguori
2011-12-01  8:33   ` Stefan Hajnoczi
2011-12-01 13:31     ` Anthony Liguori
2011-12-01 15:51   ` Gerd Hoffmann
2011-12-02  1:03     ` Anthony Liguori
2011-12-02 12:19       ` Gerd Hoffmann
2011-12-01 16:14   ` Kevin Wolf
2011-12-02  1:05     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 03/18] qom: introduce root device Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 04/18] qdev: provide an interface to return canonical path from root Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 05/18] qdev: provide a path resolution Anthony Liguori
2011-12-01 10:24   ` Stefan Hajnoczi
2011-12-01 13:34     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 06/18] qom: add child properties (composition) Anthony Liguori
2011-12-02 11:54   ` Kevin Wolf
2011-12-02 14:54     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 07/18] qom: add link properties Anthony Liguori
2011-12-01 10:55   ` Stefan Hajnoczi
2011-12-01 13:40     ` Anthony Liguori
2011-12-01 11:21   ` Avi Kivity
2011-12-01 11:35     ` Stefan Hajnoczi
2011-12-01 12:34       ` Avi Kivity
2011-12-01 13:47         ` Anthony Liguori
2011-12-01 13:50           ` Avi Kivity
2011-12-01 14:56             ` Anthony Liguori
2011-12-01 13:44     ` Anthony Liguori
2011-12-01 14:03       ` Avi Kivity
2011-12-01 14:53         ` Anthony Liguori
2011-12-01 15:00           ` Avi Kivity
2011-12-01 15:10             ` Anthony Liguori
2011-12-01 15:03           ` Gerd Hoffmann
2011-12-01 15:13             ` Avi Kivity
2011-12-01 15:14             ` Anthony Liguori
2011-12-02 12:15   ` Kevin Wolf
2011-12-02 14:57     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 08/18] qapi: allow a 'gen' key to suppress code generation Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 09/18] qmp: add qom-list command Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 10/18] qom: qom_{get,set} monitor commands Anthony Liguori
2011-12-01 11:04   ` Stefan Hajnoczi
2011-12-01 13:35     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 11/18] qdev: add explicitly named devices to the root complex Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 12/18] dev: add an anonymous peripheral container Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 13/18] rtc: make piix3 set the rtc as a child Anthony Liguori
2011-12-01 11:07   ` Stefan Hajnoczi
2011-12-01 13:35     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 14/18] rtc: add a dynamic property for retrieving the date Anthony Liguori
2011-12-01 15:46   ` Gerd Hoffmann
2011-12-02  1:19     ` Anthony Liguori
2011-12-02 12:35       ` Gerd Hoffmann
2011-12-02 13:20         ` Anthony Liguori
2011-12-02 13:34           ` Gerd Hoffmann
2011-12-02 15:05             ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 15/18] qom: optimize qdev_get_canonical_path using a parent link Anthony Liguori
2011-12-01 11:21   ` Stefan Hajnoczi
2011-12-01 13:38     ` Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 16/18] Make qmp.py easier to use Anthony Liguori
2011-11-30 21:03 ` [Qemu-devel] [PATCH 17/18] Add test tools Anthony Liguori
2011-12-01 11:26   ` Stefan Hajnoczi
2011-12-01 13:39     ` Anthony Liguori
2011-12-01 14:03       ` Stefan Hajnoczi
2011-11-30 21:03 ` [Qemu-devel] [PATCH 18/18] qdev: split out QOM functions to separate files Anthony Liguori
2011-11-30 22:54 ` [Qemu-devel] [PATCH 00/18] qom: dynamic properties and composition tree Anthony Liguori
2011-12-01 14:20 ` Avi Kivity
2011-12-01 14:42   ` Anthony Liguori
2011-12-01 14:48     ` Avi Kivity
2011-12-01 15:01       ` Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EDC8BE7.1000402@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=lcapitulino@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.