From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33634) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xc3K6-0002uY-M6 for qemu-devel@nongnu.org; Wed, 08 Oct 2014 22:19:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xc3K1-0006nU-Rg for qemu-devel@nongnu.org; Wed, 08 Oct 2014 22:19:22 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:48808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xc3K1-0006m5-39 for qemu-devel@nongnu.org; Wed, 08 Oct 2014 22:19:17 -0400 Message-ID: <5435EF21.3040606@huawei.com> Date: Thu, 9 Oct 2014 10:12:49 +0800 From: zhanghailiang MIME-Version: 1.0 References: <1412068849-14848-1-git-send-email-zhang.zhanghailiang@huawei.com> <1412068849-14848-3-git-send-email-zhang.zhanghailiang@huawei.com> <543550E5.1090707@redhat.com> In-Reply-To: <543550E5.1090707@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 2/2] dump: Turn some functions to void to make code cleaner List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, luonengjun@huawei.com, armbru@redhat.com, lcapitulino@redhat.com On 2014/10/8 22:57, Eric Blake wrote: > On 09/30/2014 03:20 AM, zhanghailiang wrote: >> Functions shouldn't return an error code and an Error object at the same time. >> Turn all these functions that returning Error object to void. >> We also judge if a function success or fail by reference to the local_err. >> >> Signed-off-by: zhanghailiang >> --- >> dump.c | 313 ++++++++++++++++++++++++++++++----------------------------------- >> 1 file changed, 143 insertions(+), 170 deletions(-) >> > > >> @@ -348,49 +326,45 @@ static int write_elf_section(DumpState *s, int type, Error **errp) >> ret = fd_write_vmcore(&shdr, shdr_size, s); >> if (ret < 0) { >> dump_error(s, "dump: failed to write section header table", errp); >> - return -1; >> + return; >> } >> - >> - return 0; >> } > > The 'return' here is not in a loop, and therefore not necessary. > >> >> -static int write_data(DumpState *s, void *buf, int length, Error **errp) >> +static void write_data(DumpState *s, void *buf, int length, Error **errp) >> { >> int ret; >> >> ret = fd_write_vmcore(buf, length, s); >> if (ret < 0) { >> dump_error(s, "dump: failed to save memory", errp); >> - return -1; >> + return; >> } >> - >> - return 0; >> } > > and again. > >> >> /* write the memroy to vmcore. 1 page per I/O. */ > > Please s/memroy/memory/ while touching this :) > > >> @@ -1706,7 +1680,6 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin, >> } else { >> create_vmcore(s, errp); >> } >> - >> g_free(s); >> } > > Looks a bit like a spurious line deletion in this hunk. > I will fix all the issues and send V8 with 'Reviewed-by' tag, Thanks very much;) > Findings are minor, so I'm fine if you add: > Reviewed-by: Eric Blake >