Stefan Weil <w...@mail.berlios.de> writes: > Am 07.02.2011 08:23, schrieb Markus Armbruster: >> Stefan Weil <w...@mail.berlios.de> 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?
Let's see the patches :)
