From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MQbTP-0002IS-Up
	for qemu-devel@nongnu.org; Tue, 14 Jul 2009 02:26:43 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MQbTL-0002Ce-O1
	for qemu-devel@nongnu.org; Tue, 14 Jul 2009 02:26:43 -0400
Received: from [199.232.76.173] (port=55900 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MQbTL-0002CY-LV
	for qemu-devel@nongnu.org; Tue, 14 Jul 2009 02:26:39 -0400
Received: from mx2.redhat.com ([66.187.237.31]:52706)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1MQbTL-0002d5-7H
	for qemu-devel@nongnu.org; Tue, 14 Jul 2009 02:26:39 -0400
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n6E6QckE021412
	for <qemu-devel@nongnu.org>; Tue, 14 Jul 2009 02:26:38 -0400
Message-ID: <4A5C251A.6050309@redhat.com>
Date: Tue, 14 Jul 2009 08:26:34 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/6] qdev/compat: compat
	property	infrastructure.
References: <1247499005-31011-1-git-send-email-kraxel@redhat.com>
	<1247499005-31011-3-git-send-email-kraxel@redhat.com>
	<20090713193652.GA10979@redhat.com>
In-Reply-To: <20090713193652.GA10979@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org

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