From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4E2i-0000SF-Vh for qemu-devel@nongnu.org; Wed, 22 Aug 2012 12:44:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4E2d-0006VU-4b for qemu-devel@nongnu.org; Wed, 22 Aug 2012 12:44:32 -0400 Message-ID: <50350C58.8060306@siemens.com> Date: Wed, 22 Aug 2012 18:44:08 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <5034FB7D.5050009@siemens.com> <503508CE.1020908@weilnetz.de> In-Reply-To: <503508CE.1020908@weilnetz.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: "qemu-trivial@nongnu.org" , "qemu-devel@nongnu.org" , BALATON Zoltan 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 >>> --- >>> 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