From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:50536)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Alexander_Barabash@mentor.com>) id 1RzUoT-0004SJ-Ew
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 10:06:11 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Alexander_Barabash@mentor.com>) id 1RzUoM-0002lv-CH
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 10:06:01 -0500
Received: from relay1.mentorg.com ([192.94.38.131]:62440)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Alexander_Barabash@mentor.com>) id 1RzUoM-0002kc-5T
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 10:05:54 -0500
Message-ID: <4F426160.4040403@mentor.com>
Date: Mon, 20 Feb 2012 17:06:08 +0200
From: Alexander Barabash <alexander_barabash@mentor.com>
MIME-Version: 1.0
References: <4F3D3F67.9050506@mentor.com> <4F3E2945.3000603@redhat.com>
	<4F40ECC6.8000603@mentor.com> <4F411D76.9020105@mentor.com>
	<4F412E06.8020104@suse.de>
In-Reply-To: <4F412E06.8020104@suse.de>
Content-Type: text/plain; charset="ISO-8859-1"; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH v2] Revised: Add
	object_property_get_child().
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: =?ISO-8859-1?Q?Andreas_F=E4rber?= <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Peter Maydell <peter.maydell@linaro.org>

On 02/19/2012 07:14 PM, Andreas Färber wrote:
> Am 19.02.2012 17:04, schrieb Alexander Barabash:
>> ...
>>      Signed-off-by: Alexander Barabash<alexander_barabash@mentor.com>
> Please use git-send-email to submit your patches: The commit message is
> unnecessarily indented and the first line is duplicated. Instead of
> "Revised: " (which v2 already indicates) the subject should mention the
> topic, here "qom: ".
>
> http://wiki.qemu.org/Contribute/SubmitAPatch

Thanks for the advice.

>
> Your patch is doing too many things at once. Please split it up into a
> series of easily digestable and bisectable patches, e.g.:
> * ..._PREFIX/SUFFIX introduction and use in _add_child/link,
> * ..._is_child/_is_link introduction and use in
> _property_del_child/_get_canonical_path/_resolve_partial_path,
> * link_type_to_type() and use in _set_link_property,
> * REPORT_OBJECT_ERROR(),
> * object_property_get_child().

OK

>> +/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to
>> FOO.  */
>> +static gchar *link_type_to_type(const gchar *type)
>> +{
>> +    return g_strndup(&type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1],
>> +                     strlen(type)
>> +                     - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1)
>> +                     - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1));
> Any reason not to use strlen()? I don't think this is a hot path, and
> repeated sizeof() - 1 does not read so nice. The alternative would be to
> hardcode the offsets.

I replaced "5" (i.e. the length "link<") with 
"sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1".
If we assume that "strlen("link<") is always optimized away,
then certainly using strlen is equivalent and looks better.

>
> Andreas
>
Alex