From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TK5xi-00049C-7a for qemu-devel@nongnu.org; Fri, 05 Oct 2012 07:20:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TK5xb-000891-LF for qemu-devel@nongnu.org; Fri, 05 Oct 2012 07:20:58 -0400 Received: from m12-12.163.com ([220.181.12.12]:40280) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TK5xa-0007rP-EN for qemu-devel@nongnu.org; Fri, 05 Oct 2012 07:20:51 -0400 Message-ID: <506EC26F.4020002@163.com> Date: Fri, 05 Oct 2012 19:20:15 +0800 From: wenchao xia 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> <506C2DDD.6090500@redhat.com> In-Reply-To: <506C2DDD.6090500@redhat.com> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V5 5/5] libqblock test example List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, eblake@redhat.com, Wenchao Xia ÓÚ 2012-10-3 20:21, Paolo Bonzini дµÀ: > 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 > OK, then I'll remove it. Do you think there is other things need to be modified?