From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44034 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmYOc-0000st-Nt for qemu-devel@nongnu.org; Mon, 07 Feb 2011 16:13:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmVFf-0003Zs-6e for qemu-devel@nongnu.org; Mon, 07 Feb 2011 12:51:52 -0500 Received: from moutng.kundenserver.de ([212.227.126.187]:64204) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmVFe-0003Zl-TO for qemu-devel@nongnu.org; Mon, 07 Feb 2011 12:51:51 -0500 Message-ID: <4D503132.3010807@mail.berlios.de> Date: Mon, 07 Feb 2011 18:51:46 +0100 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *)) References: <4D4D612F.2010904@mail.berlios.de> <4D4DC570.8080204@mail.berlios.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Blue Swirl , QEMU Developers Am 07.02.2011 08:23, schrieb Markus Armbruster: > Stefan Weil 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