[Qemu-devel] [PATCH] Fix console_write_ch on 64-bit big-endian hosts

[Qemu-devel] [PATCH] Fix console_write_ch on 64-bit big-endian hosts

[Anthony Liguori]

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>

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 *);
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] Fix console_write_ch on 64-bit big-endian hosts

[andrzej zaborowski]

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

Re: [Qemu-devel] [PATCH] Fix console_write_ch on 64-bit big-endian hosts

[Anthony Liguori]

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
>    

Re: [Qemu-devel] [PATCH] Fix console_write_ch on 64-bit big-endian hosts

[andrzej zaborowski]

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