From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:57471)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TJNxk-00021r-BH
	for qemu-devel@nongnu.org; Wed, 03 Oct 2012 08:22:08 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TJNxj-0002m9-Cn
	for qemu-devel@nongnu.org; Wed, 03 Oct 2012 08:22:04 -0400
Received: from mx1.redhat.com ([209.132.183.28]:62169)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1TJNxj-0002jK-4W
	for qemu-devel@nongnu.org; Wed, 03 Oct 2012 08:22:03 -0400
Message-ID: <506C2DDD.6090500@redhat.com>
Date: Wed, 03 Oct 2012 14:21:49 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1348906418-16471-1-git-send-email-xiawenc@linux.vnet.ibm.com>
	<1348906418-16471-6-git-send-email-xiawenc@linux.vnet.ibm.com>
	<506C2206.7030809@163.com>
In-Reply-To: <506C2206.7030809@163.com>
Content-Type: text/plain; charset=GB2312
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH V5 5/5] libqblock test example
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: wenchao xia <xiaxia347os@163.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, eblake@redhat.com, Wenchao Xia <xiawenc@linux.vnet.ibm.com>

Il 03/10/2012 13:31, wenchao xia ha scritto:
>>> +const uint64_t *qb_get_virt_size(const QBlockStaticInfo *info)
>>> +{
>>> +    return info->member_addr->virt_size;
>>
>> Please change this to:
>>
>>     QBlockStaticInfoAddr addrs;
>>     qb_setup_info_addr(info, &addrs);
>>     return *addrs->virt_size;
>>
>> and similarly for the others.
>>
>> QBlockStaticInfoAddr should not be a public struct.  I'm sorry if this
>> wasn't clear.
>>
>> I'll review the rest on Monday.
>>
>> Paolo
>>
>   Hi, QBlockStaticInfoAddr was declared as a pointer, user can't see
> what it really is. It is actually defined in internal header files.

Yes, but even the type should be hidden.  It should be purely an
implementation detail, at least for now, to hide the switch in a single
function.

Paolo