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

[Stefan Hajnoczi]

On Wed, Aug 22, 2012 at 06:44:08PM +0200, 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 <address@hidden>
> >>> ---
> >>>   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.

I have updated the commit message to "console: Clean up bytes per pixel
calculation".

Stefan