|
From: | Stefan Weil |
Subject: | Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *)) |
Date: | Mon, 07 Feb 2011 18:51:46 +0100 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20101226 Iceowl/1.0b1 Icedove/3.0.11 |
Am 07.02.2011 08:23, schrieb Markus Armbruster:
Stefan Weil <address@hidden> writes:Am 05.02.2011 16:35, schrieb Blue Swirl:[...]The patch changes also signed longs to uintptr_t. That could introduce regressions, so please use signed/unsigned as original.I changed the code manually, and there was only one location where signed/unsigned made a difference. That single case was an int parameter passed in a void pointer, and I used intptr_t there. I had the impression that in the current code (long) was sometimes used because it is shorter than (unsigned long) :-) As long as changes are made manually with the necessary care, I'd recommend using uintptr_t where possible.I'd recommend not to mix up the intptr portability clean up with the signedness cleanup. Much easier to review separately. Moreover, cleaning up signedness changes generated code, while cleaning up the types shouldn't (except on hosts where the code doesn't work). Testable, just don't forget to strip the debug info. [...]
Markus, your recommendation is ok for modifications which change the generated code or which need more context for the review. I don't think that you will have any problem with reviewing "signedness changes" like these ones: -#define saddr(x) (uint8_t *)(long)(x) -#define laddr(x) (uint8_t *)(long)(x) +#define saddr(x) (uint8_t *)(uintptr_t)(x) +#define laddr(x) (uint8_t *)(uintptr_t)(x)Neither of these changes changes the binary code for the commonly used hosts,
and the patch does not need further context for the review. I intend to split my patch in three parts:* one for tcg_gen_exit_tb calls which will be modified as Blue Swirl has suggested
* one for the parameter passing of a signed value via pointer* one for the rest which contains only a handful of trivial "signedness changes",
all following the same pattern like the example given above Is that ok? Regards, Stefan
[Prev in Thread] | Current Thread | [Next in Thread] |