From: Gerd Hoffmann <kraxel@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/6] qdev/compat: compat property infrastructure.
Date: Tue, 14 Jul 2009 08:26:34 +0200 [thread overview]
Message-ID: <4A5C251A.6050309@redhat.com> (raw)
In-Reply-To: <20090713193652.GA10979@redhat.com>
On 07/13/09 21:36, Michael S. Tsirkin wrote:
> Some coding style comments
>> + return;
>> + for (prop = compat_props; prop->driver != NULL; prop++) {
>
> != NULL not needed in if
>
>> + if (strcmp(dev->info->name, prop->driver) != 0)
>
> != 0 not needed in if
I still prefer to have it explicitly written as it makes the code more
readable IMHO.
>> void qdev_prop_set_defaults(DeviceState *dev, Property *props);
>>
>> +void qdev_register_compat_props(CompatProperty *props);
>
> qedev_set_compat_props might be a better name.
>
>> +void qdev_prop_set_compat(DeviceState *dev);
>
> qdev_parse_compat_props might be a better name.
Disagree on both. qdev_prop_set_compat intentionally follows the name
convention of the other qdev_prop_set_* functions. The
qdev_register_compat_props intentionally isn't named something with
"set" to avoid confusion with the other ones because it doesn't actually
set properties on devices. "register" maybe isn't the best idea, I'm
open to better suggestions.
Fixed the other ones.
cheers,
Gerd
next prev parent reply other threads:[~2009-07-14 6:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-13 15:29 [Qemu-devel] [PATCH v3 0/7] qdev: compat properties Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 1/6] cleanup: drop unused struct elements from VirtIOPCIProxy Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 2/6] qdev/compat: compat property infrastructure Gerd Hoffmann
2009-07-13 19:36 ` Michael S. Tsirkin
2009-07-14 6:26 ` Gerd Hoffmann [this message]
2009-07-14 9:09 ` [Qemu-devel] " Juan Quintela
2009-07-14 14:19 ` Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 3/6] qdev/compat: add pc-0.10 machine type Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 4/6] qdev/compat: virtio-blk-pci 0.10 compatibility Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 5/6] qdev/compat: virtio-console-pci " Gerd Hoffmann
2009-07-13 15:30 ` [Qemu-devel] [PATCH 6/6] qdev/compat: virtio-net-pci " Gerd Hoffmann
2009-07-13 15:47 ` [Qemu-devel] [PATCH v3 0/7] qdev: compat properties Anthony Liguori
2009-07-13 18:42 ` Gerd Hoffmann
-- strict thread matches above, loose matches on Subject: below --
2009-07-15 11:48 [Qemu-devel] [PATCH 0/6] " Gerd Hoffmann
2009-07-15 11:48 ` [Qemu-devel] [PATCH 2/6] qdev/compat: compat property infrastructure Gerd Hoffmann
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=4A5C251A.6050309@redhat.com \
--to=kraxel@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).