Message ID | 1377269838-13347-1-git-send-email-yongbok.kim@imgtec.com |
---|---|
State | New |
Headers | show |
Am 23.08.2013 16:57, schrieb Yongbok Kim: > A parenthesis placed inappropriately caused displaying > wrong memory size bigger than 4GB. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > hw/mips/mips_malta.c | 2 +- > hw/mips/mips_r4k.c | 2 +- > hw/ppc/mac_oldworld.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) Thanks for fixing this even beyond MIPS machines! In theory I would've preferred a patch using the correct format string and dropping the unsigned int cast completely, but our RAM_ADDR_FMT uses hexadecimal format, so: Reviewed-by: Andreas Färber <afaerber@suse.de> > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index f8d064c..23ac1ca 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); Seeing that in the Malta case your other patch will trivially conflict with this bugfix, would you have time to turn it into a series of three patches, this being the first and adding "Cc: qemu-stable@nongnu.org" to its commit message for inclusion in 1.6.1, then an additional patch replacing fprintf(stderr, "qemu: ...\n", ...) with error_report("...", ...) (note no trailing \n and it will also automatically prefix the right qemu-system-mips* executable name) and finally your 2 GiB patch? Regards, Andreas > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); > diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c > index 044f232..e8108ac 100644 > --- a/hw/mips/mips_r4k.c > +++ b/hw/mips/mips_r4k.c > @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size); > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index 42bb9d5..d7d1758 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args) > if (ram_size > (2047 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > >
On 23.08.2013, at 15:57, Yongbok Kim wrote: > A parenthesis placed inappropriately caused displaying > wrong memory size bigger than 4GB. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> Acked-by: Alexander Graf <agraf@suse.de> I think this should easily go in through the qemu-trivial queue. Alex > --- > hw/mips/mips_malta.c | 2 +- > hw/mips/mips_r4k.c | 2 +- > hw/ppc/mac_oldworld.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index f8d064c..23ac1ca 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); > diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c > index 044f232..e8108ac 100644 > --- a/hw/mips/mips_r4k.c > +++ b/hw/mips/mips_r4k.c > @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size); > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index 42bb9d5..d7d1758 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args) > if (ram_size > (2047 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > > -- > 1.7.4 > >
On 23.08.2013, at 17:20, Andreas Färber wrote: > Am 23.08.2013 16:57, schrieb Yongbok Kim: >> A parenthesis placed inappropriately caused displaying >> wrong memory size bigger than 4GB. >> >> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> >> --- >> hw/mips/mips_malta.c | 2 +- >> hw/mips/mips_r4k.c | 2 +- >> hw/ppc/mac_oldworld.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) > > Thanks for fixing this even beyond MIPS machines! > > In theory I would've preferred a patch using the correct format string > and dropping the unsigned int cast completely, but our RAM_ADDR_FMT uses > hexadecimal format, so: > > Reviewed-by: Andreas Färber <afaerber@suse.de> > >> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c >> index f8d064c..23ac1ca 100644 >> --- a/hw/mips/mips_malta.c >> +++ b/hw/mips/mips_malta.c >> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) >> if (ram_size > (256 << 20)) { >> fprintf(stderr, >> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", >> - ((unsigned int)ram_size / (1 << 20))); >> + (unsigned int)(ram_size / (1 << 20))); > > Seeing that in the Malta case your other patch will trivially conflict > with this bugfix, would you have time to turn it into a series of three > patches, this being the first and adding "Cc: qemu-stable@nongnu.org" to > its commit message for inclusion in 1.6.1, then an additional patch > replacing fprintf(stderr, "qemu: ...\n", ...) with error_report("...", > ...) (note no trailing \n and it will also automatically prefix the > right qemu-system-mips* executable name) and finally your 2 GiB patch? Ah, saw this comment too late. Andreas' suggestions is obviously even better :). Alex
Because this patch is not the critical, probably I can re-send this and the additional patches (Andreas' suggestion) as trivial patches when the 2GiB patch applied. Regards, Yongbok -----Original Message----- From: Alexander Graf [mailto:agraf@suse.de] Sent: 25 August 2013 17:12 To: Andreas Färber Cc: Yongbok Kim; qemu-devel@nongnu.org; Leon Alrae; aurelien@aurel32.net; critian.cuna@imgtec.com Subject: Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size On 23.08.2013, at 17:20, Andreas Färber wrote: > Am 23.08.2013 16:57, schrieb Yongbok Kim: >> A parenthesis placed inappropriately caused displaying wrong memory >> size bigger than 4GB. >> >> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> >> --- >> hw/mips/mips_malta.c | 2 +- >> hw/mips/mips_r4k.c | 2 +- >> hw/ppc/mac_oldworld.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) > > Thanks for fixing this even beyond MIPS machines! > > In theory I would've preferred a patch using the correct format string > and dropping the unsigned int cast completely, but our RAM_ADDR_FMT > uses hexadecimal format, so: > > Reviewed-by: Andreas Färber <afaerber@suse.de> > >> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index >> f8d064c..23ac1ca 100644 >> --- a/hw/mips/mips_malta.c >> +++ b/hw/mips/mips_malta.c >> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) >> if (ram_size > (256 << 20)) { >> fprintf(stderr, >> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", >> - ((unsigned int)ram_size / (1 << 20))); >> + (unsigned int)(ram_size / (1 << 20))); > > Seeing that in the Malta case your other patch will trivially conflict > with this bugfix, would you have time to turn it into a series of > three patches, this being the first and adding "Cc: > qemu-stable@nongnu.org" to its commit message for inclusion in 1.6.1, > then an additional patch replacing fprintf(stderr, "qemu: ...\n", ...) > with error_report("...", > ...) (note no trailing \n and it will also automatically prefix the > right qemu-system-mips* executable name) and finally your 2 GiB patch? Ah, saw this comment too late. Andreas' suggestions is obviously even better :). Alex
On Fri, Aug 23, 2013 at 03:57:18PM +0100, Yongbok Kim wrote: > A parenthesis placed inappropriately caused displaying > wrong memory size bigger than 4GB. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > hw/mips/mips_malta.c | 2 +- > hw/mips/mips_r4k.c | 2 +- > hw/ppc/mac_oldworld.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index f8d064c..23ac1ca 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); > diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c > index 044f232..e8108ac 100644 > --- a/hw/mips/mips_r4k.c > +++ b/hw/mips/mips_r4k.c > @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args) > if (ram_size > (256 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } > memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size); > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index 42bb9d5..d7d1758 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args) > if (ram_size > (2047 << 20)) { > fprintf(stderr, > "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n", > - ((unsigned int)ram_size / (1 << 20))); > + (unsigned int)(ram_size / (1 << 20))); > exit(1); > } For the MIPS part: Acked-by: Aurelien Jarno <aurelien@aurel32.net> But as already said, it's probably better to wait for the Malta 2GB patch to be merged first and send it to the trivial patches queue.
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index f8d064c..23ac1ca 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) if (ram_size > (256 << 20)) { fprintf(stderr, "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", - ((unsigned int)ram_size / (1 << 20))); + (unsigned int)(ram_size / (1 << 20))); exit(1); } memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c index 044f232..e8108ac 100644 --- a/hw/mips/mips_r4k.c +++ b/hw/mips/mips_r4k.c @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args) if (ram_size > (256 << 20)) { fprintf(stderr, "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", - ((unsigned int)ram_size / (1 << 20))); + (unsigned int)(ram_size / (1 << 20))); exit(1); } memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size); diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index 42bb9d5..d7d1758 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args) if (ram_size > (2047 << 20)) { fprintf(stderr, "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n", - ((unsigned int)ram_size / (1 << 20))); + (unsigned int)(ram_size / (1 << 20))); exit(1); }
A parenthesis placed inappropriately caused displaying wrong memory size bigger than 4GB. Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- hw/mips/mips_malta.c | 2 +- hw/mips/mips_r4k.c | 2 +- hw/ppc/mac_oldworld.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)