Re: [Qemu-devel] [PATCH] target_posix_types.h

[Fabrice Bellard <fabrice@bellard.org>]

Thayne Harbaugh wrote:
> On Wed, 2007-11-14 at 19:32 +0100, Fabrice Bellard wrote:
>> Thayne Harbaugh wrote:
>>> This patch, 44_target_posix_types.patch provides target specific posix
>>> types.  These types improve target structure creation, code similarity
>>> to kernel code and improve type casting for assignment between target
>>> and host.
>> Why is it needed ?
> 
> 
> It's not *necessary*, but it makes code more readable and it simplifies
> having to always check for what a typedef on a target might map to.  It
> makes target structures more comparable to their structures in the
> kernel.
> 
> A simple example:
> 
> struct target_timeval {
>     abi_long tv_sec;
>     abi_long tv_usec;
> };
> 
> vs.
> 
> struct target_timeval {
>     target_time_t tv_sec;
>     target_suseconds_t tv_usec;
> };
> 
> 
> It also makes type conversion between target and host more obvious.
> 
> It also means that more code can be shared rather than #ifdef'ed when
> targets differ on their base definitions.
> 
> It's just another level of abstraction, we can always stick with
> abi_long, abi_ulong, etc..  I've just noticed that size and sign
> handling when converting between target and host are a common source of
> errors and this simplifies things.  We use it in all our patches and it
> has helped simplify and fix errors.

I don't like adding levels of abstraction unless I am really forced. I 
think these types makes the code more difficult to understand without 
real added value. In particular, it does not help for sign conversions.

Fabrice.