From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35052) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZttWU-0007E3-3L for qemu-devel@nongnu.org; Wed, 04 Nov 2015 03:34:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZttWT-00045E-7x for qemu-devel@nongnu.org; Wed, 04 Nov 2015 03:34:26 -0500 References: <1446553829-28463-1-git-send-email-caoj.fnst@cn.fujitsu.com> <5638AB2B.5060700@msgid.tls.msk.ru> From: Cao jin Message-ID: <5639C307.7030700@cn.fujitsu.com> Date: Wed, 4 Nov 2015 16:34:15 +0800 MIME-Version: 1.0 In-Reply-To: <5638AB2B.5060700@msgid.tls.msk.ru> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] set_memory_options: remove code that make no sense List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev , qemu-devel@nongnu.org Cc: qemu-trivial@nongnu.org hi Michael, Thanks for your explanation that make me realized I am wrong about my patch:-[ ...So, forget it. On 11/03/2015 08:40 PM, Michael Tokarev wrote: > 03.11.2015 15:30, Cao jin wrote: >> Signed-off-by: Cao jin >> --- >> vl.c | 9 --------- >> 1 file changed, 9 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index f5f7c3f..13f2c8b 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2860,11 +2860,6 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size, >> sz = 0; >> mem_str = qemu_opt_get(opts, "size"); >> if (mem_str) { >> - if (!*mem_str) { >> - error_report("missing 'size' option value"); >> - exit(EXIT_FAILURE); >> - } > > I'm not sure this one is bad or good, it is indeed possible > to specify no value for size=, but if we're to check that, > we'd have to add such checks everywhere. > Yup..I missed to test "-m size=", just test several case with format "-m ###", and didn`t read get_opt_value throughly, or else will find it can output a string like "\0" which I never encountered before;) > But the next one... > >> sz = qemu_opt_get_size(opts, "size", ram_size); >> >> /* Fix up legacy suffix-less format */ >> @@ -2886,10 +2881,6 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size, >> >> sz = QEMU_ALIGN_UP(sz, 8192); >> ram_size = sz; >> - if (ram_size != sz) { >> - error_report("ram size too large"); >> - exit(EXIT_FAILURE); >> - } > > is definitely wrong. > > sz is uint64_t, while ram_size is ram_addr_t which is > either uint64_t or uintptr_t. Until it is fixed to > always be 64bits, the above code makes (some) sense. > yes,I am careless... maybe #ifdef CONFIG_XEN_BACKEND if (ram_size != sz) { error_report("ram size too large"); exit(EXIT_FAILURE); } #endif is better, but no big deal;) anyway, forget this patch:P > Thanks, > > /mjt > > -- Yours Sincerely, Cao Jin