Message ID | 1446553829-28463-1-git-send-email-caoj.fnst@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
03.11.2015 15:30, Cao jin wrote: > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > 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. 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. Thanks, /mjt
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 <caoj.fnst@cn.fujitsu.com> >> --- >> 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 > >
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); - } - 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); - } /* store value for the future use */ qemu_opt_set_number(opts, "size", ram_size, &error_abort);
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- vl.c | 9 --------- 1 file changed, 9 deletions(-)