| Message ID | 1275505133-3734-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 2 June 2010 20:58, Anthony Liguori <aliguori@us.ibm.com> wrote: > Currently, console_ch_t is defined as an unsigned long. However, immediately > after it's definition, we treat it as a uint32_t *. This will work on a little > endian system because of the way bits are layed out but will fail miserably > on big endian hosts. It seems that what this really tries to do is like *dest = leul_to_cpu(v) from bswap.h? (Or cpu_to_leul.. quite difficult to wrap my head around it..) Cheers
On 06/02/2010 02:31 PM, andrzej zaborowski wrote: > Hi, > > On 2 June 2010 20:58, Anthony Liguori<aliguori@us.ibm.com> wrote: > >> Currently, console_ch_t is defined as an unsigned long. However, immediately >> after it's definition, we treat it as a uint32_t *. This will work on a little >> endian system because of the way bits are layed out but will fail miserably >> on big endian hosts. >> > It seems that what this really tries to do is like *dest = > leul_to_cpu(v) from bswap.h? (Or cpu_to_leul.. quite difficult to wrap > my head around it..) > Yeah, I think it possibly should be a leul_to_cpu with the function signature changed to take an unsigned long. But one thing I'm very confused about is why console_ch_t is an unsigned long vs. a uint32.. Regards, Anthony Liguori > Cheers >
On 2 June 2010 22:32, Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote: > On 06/02/2010 02:31 PM, andrzej zaborowski wrote: >> It seems that what this really tries to do is like *dest = >> leul_to_cpu(v) from bswap.h? (Or cpu_to_leul.. quite difficult to wrap >> my head around it..) >> > > Yeah, I think it possibly should be a leul_to_cpu with the function > signature changed to take an unsigned long. > > But one thing I'm very confused about is why console_ch_t is an unsigned > long vs. a uint32.. I guess it is to be same as the curses' chtype, so they can be used interchangeably. curses.c is literally the only user of console_ch_t so it kind of makes sense. The problem is that console.h tries to abstract the type of output device curses is, and fails. And the abstraction is unnecessary because curses is the only such interface there. Cheers
diff --git a/console.h b/console.h index cac959f..ddd1bbf 100644 --- a/console.h +++ b/console.h @@ -326,9 +326,11 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds) typedef unsigned long console_ch_t; static inline void console_write_ch(console_ch_t *dest, uint32_t ch) { + uint32_t p; if (!(ch & 0xff)) ch |= ' '; - cpu_to_le32wu((uint32_t *) dest, ch); + cpu_to_le32wu(&p, ch); + *dest = p; } typedef void (*vga_hw_update_ptr)(void *);
Currently, console_ch_t is defined as an unsigned long. However, immediately after it's definition, we treat it as a uint32_t *. This will work on a little endian system because of the way bits are layed out but will fail miserably on big endian hosts. This patch fixes the code to do the correct thing. This addresses https://bugs.launchpad.net/qemu/+bug/568614 Reported-by: Devin J. Pohly Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>