From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v3 00/32] implement vNVDIMM Date: Tue, 13 Oct 2015 13:52:51 +0800 Message-ID: <561C9C33.1030405@linux.intel.com> References: <1444535584-18220-1-git-send-email-guangrong.xiao@linux.intel.com> <20151012144113-mutt-send-email-mst@redhat.com> <561C96CC.40702@linux.intel.com> <20151013084304-mutt-send-email-mst@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: pbonzini@redhat.com, imammedo@redhat.com, gleb@kernel.org, mtosatti@redhat.com, stefanha@redhat.com, rth@twiddle.net, ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org To: "Michael S. Tsirkin" Return-path: Received: from mga09.intel.com ([134.134.136.24]:29391 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751452AbbJMF7C (ORCPT ); Tue, 13 Oct 2015 01:59:02 -0400 In-Reply-To: <20151013084304-mutt-send-email-mst@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/13/2015 01:57 PM, Michael S. Tsirkin wrote: > On Tue, Oct 13, 2015 at 01:29:48PM +0800, Xiao Guangrong wrote: >> >> >> On 10/12/2015 07:55 PM, Michael S. Tsirkin wrote: >>> On Sun, Oct 11, 2015 at 11:52:32AM +0800, Xiao Guangrong wrote: >>>> Changelog in v3: >>>> There is huge change in this version, thank Igor, Stefan, Paolo, Eduardo, >>>> Michael for their valuable comments, the patchset finally gets better shape. >>> >>> Thanks! >>> This needs some changes in coding style, and more comments, to >>> make it easier to maintain going forward. >> >> Thanks for your review, Michael. I have learned lots of thing from >> your comments. >> >>> >>> High level comments - I didn't point out all instances, >>> please go over code and locate them yourself. >>> I focused on acpi code in this review. >> >> Okay, will do. >> >>> >>> - fix coding style violations, prefix eveything with nvdimm_ etc >> >> Actually i did not pay attention on naming the stuff which is only internally >> used. Thank you for pointing it out and will fix it in next version. >> >>> - in apci code, avoid manual memory management/complex pointer math >> >> I am not very good at ACPI ASL/AML, could you please more detail? > > It's about C. > > For example: > Foo *foo = acpi_data_push(table, sizeof *foo); > Bar *foo = acpi_data_push(table, sizeof *bar); > is pretty obviously safe, and it doesn't require you to do any > calculations. > char *buf = acpi_data_push(table, sizeof *foo + sizeof *bar); > is worse, now you need: > Bar *bar = (Bar *)(buf + sizeof *foo); > which will corrupt memory if you get the size wrong in push. Ah, got it. :)