Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-24 Thread Stefan Hajnoczi 
On Wed, Aug 22, 2012 at 05:19:42PM +0200, BALATON Zoltan wrote:
 Division with round up is the correct way to compute this even if the
 only case where division with round down gives incorrect result is
 probably 15 bpp. This case was explicitely patched up in one of these
 functions but was unhandled in the other. (I'm not sure about setting
 16 bpp for the 15bpp case either but I left that there for now.)
 
 Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
 ---
  console.c |5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
  v2: Use DIV_ROUND_UP and extended commit message

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

[Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-22 Thread BALATON Zoltan 

Division with round up is the correct way to compute this even if the
only case where division with round down gives incorrect result is
probably 15 bpp. This case was explicitely patched up in one of these
functions but was unhandled in the other. (I'm not sure about setting
16 bpp for the 15bpp case either but I left that there for now.)

Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
---
 console.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

 v2: Use DIV_ROUND_UP and extended commit message

diff --git a/console.c b/console.c
index 4525cc7..9df1701 100644
--- a/console.c
+++ b/console.c
@@ -1612,7 +1612,7 @@ PixelFormat qemu_different_endianness_pixelformat(int bpp)
 memset(pf, 0x00, sizeof(PixelFormat));

 pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
 pf.depth = bpp == 32 ? 24 : bpp;

 switch (bpp) {
@@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
 memset(pf, 0x00, sizeof(PixelFormat));

 pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
 pf.depth = bpp == 32 ? 24 : bpp;

 switch (bpp) {
 case 15:
 pf.bits_per_pixel = 16;
-pf.bytes_per_pixel = 2;
 pf.rmask = 0x7c00;
 pf.gmask = 0x03E0;
 pf.bmask = 0x001F;
--
1.7.10

Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-22 Thread Jan Kiszka 
On 2012-08-22 17:19, BALATON Zoltan wrote:
 Division with round up is the correct way to compute this even if the
 only case where division with round down gives incorrect result is
 probably 15 bpp. This case was explicitely patched up in one of these
 functions but was unhandled in the other. (I'm not sure about setting
 16 bpp for the 15bpp case either but I left that there for now.)
 
 Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
 ---
  console.c |5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
  v2: Use DIV_ROUND_UP and extended commit message
 
 diff --git a/console.c b/console.c
 index 4525cc7..9df1701 100644
 --- a/console.c
 +++ b/console.c
 @@ -1612,7 +1612,7 @@ PixelFormat
 qemu_different_endianness_pixelformat(int bpp)
  memset(pf, 0x00, sizeof(PixelFormat));
 
  pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
  pf.depth = bpp == 32 ? 24 : bpp;
 
  switch (bpp) {
 @@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
  memset(pf, 0x00, sizeof(PixelFormat));
 
  pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
  pf.depth = bpp == 32 ? 24 : bpp;
 
  switch (bpp) {
  case 15:
  pf.bits_per_pixel = 16;
 -pf.bytes_per_pixel = 2;

Removed unintentionally?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-22 Thread Stefan Weil 

Am 22.08.2012 17:32, schrieb Jan Kiszka:

On 2012-08-22 17:19, BALATON Zoltan wrote:

Division with round up is the correct way to compute this even if the
only case where division with round down gives incorrect result is
probably 15 bpp. This case was explicitely patched up in one of these
functions but was unhandled in the other. (I'm not sure about setting
16 bpp for the 15bpp case either but I left that there for now.)

Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
---
  console.c |5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

  v2: Use DIV_ROUND_UP and extended commit message

diff --git a/console.c b/console.c
index 4525cc7..9df1701 100644
--- a/console.c
+++ b/console.c
@@ -1612,7 +1612,7 @@ PixelFormat
qemu_different_endianness_pixelformat(int bpp)
  memset(pf, 0x00, sizeof(PixelFormat));

  pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
  pf.depth = bpp == 32 ? 24 : bpp;

  switch (bpp) {
@@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
  memset(pf, 0x00, sizeof(PixelFormat));

  pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
  pf.depth = bpp == 32 ? 24 : bpp;

  switch (bpp) {
  case 15:
  pf.bits_per_pixel = 16;
-pf.bytes_per_pixel = 2;

Removed unintentionally?

Jan


15 bpp no longer needs special handling because
the computation above is fixed by the same patch,
so the removal is correct.

Stefan

Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-22 Thread Jan Kiszka 
On 2012-08-22 18:29, Stefan Weil wrote:
 Am 22.08.2012 17:32, schrieb Jan Kiszka:
 On 2012-08-22 17:19, BALATON Zoltan wrote:
 Division with round up is the correct way to compute this even if the
 only case where division with round down gives incorrect result is
 probably 15 bpp. This case was explicitely patched up in one of these
 functions but was unhandled in the other. (I'm not sure about setting
 16 bpp for the 15bpp case either but I left that there for now.)

 Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
 ---
   console.c |5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)

   v2: Use DIV_ROUND_UP and extended commit message

 diff --git a/console.c b/console.c
 index 4525cc7..9df1701 100644
 --- a/console.c
 +++ b/console.c
 @@ -1612,7 +1612,7 @@ PixelFormat
 qemu_different_endianness_pixelformat(int bpp)
   memset(pf, 0x00, sizeof(PixelFormat));

   pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
   pf.depth = bpp == 32 ? 24 : bpp;

   switch (bpp) {
 @@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
   memset(pf, 0x00, sizeof(PixelFormat));

   pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
   pf.depth = bpp == 32 ? 24 : bpp;

   switch (bpp) {
   case 15:
   pf.bits_per_pixel = 16;
 -pf.bytes_per_pixel = 2;
 Removed unintentionally?

 Jan
 
 15 bpp no longer needs special handling because
 the computation above is fixed by the same patch,
 so the removal is correct.

Ok, actually starting to look at this code: This is apparently a code
cleanup, not a bug fix like the subject suggests. No valid input for
could have actually triggered the round-up issue. Please declare it as
such. Bonus for abort() on the invalid default cases.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel

2012-08-22 Thread Jan Kiszka 
On 2012-08-22 18:44, Jan Kiszka wrote:
 On 2012-08-22 18:29, Stefan Weil wrote:
 Am 22.08.2012 17:32, schrieb Jan Kiszka:
 On 2012-08-22 17:19, BALATON Zoltan wrote:
 Division with round up is the correct way to compute this even if the
 only case where division with round down gives incorrect result is
 probably 15 bpp. This case was explicitely patched up in one of these
 functions but was unhandled in the other. (I'm not sure about setting
 16 bpp for the 15bpp case either but I left that there for now.)

 Signed-off-by: BALATON Zoltan bala...@eik.bme.hu
 ---
   console.c |5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)

   v2: Use DIV_ROUND_UP and extended commit message

 diff --git a/console.c b/console.c
 index 4525cc7..9df1701 100644
 --- a/console.c
 +++ b/console.c
 @@ -1612,7 +1612,7 @@ PixelFormat
 qemu_different_endianness_pixelformat(int bpp)
   memset(pf, 0x00, sizeof(PixelFormat));

   pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
   pf.depth = bpp == 32 ? 24 : bpp;

   switch (bpp) {
 @@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
   memset(pf, 0x00, sizeof(PixelFormat));

   pf.bits_per_pixel = bpp;
 -pf.bytes_per_pixel = bpp / 8;
 +pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
   pf.depth = bpp == 32 ? 24 : bpp;

   switch (bpp) {
   case 15:
   pf.bits_per_pixel = 16;
 -pf.bytes_per_pixel = 2;
 Removed unintentionally?

 Jan

 15 bpp no longer needs special handling because
 the computation above is fixed by the same patch,
 so the removal is correct.
 
 Ok, actually starting to look at this code: This is apparently a code
 cleanup, not a bug fix like the subject suggests. No valid input for
 could have actually triggered the round-up issue. Please declare it as
 such. Bonus for abort() on the invalid default cases.

Well, do not abort on bpp == 0, the curses case.

Also, qemu_different_endianness_pixelformat is currently only called
with 16 or 32 bpp (see vga.c), but only handles 24 and 32. Fishy...

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux