Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range

[Andreas Färber]

Am 22.08.2014 15:34, schrieb Peter Maydell:
> On 22 August 2014 14:27, Andreas Färber <address@hidden> wrote:
>> Am 22.08.2014 14:09, schrieb Laurent Vivier:
>>> as in the kernel timer_t is an "int" (as said PMM), you should cast to
>>> "int" to remove garbage on 64bit hosts and check sign ...
>>
>> So maybe that's the bug Alex was trying to fix downstream with the use
>> of unsigned types?
> 
> I imagine the reason the SuSE tree switches to abi_ulong
> for the arg* is that it fixes a bunch of bugs we have where we're
> incorrectly casting a (probably 32 bit) abi_long to a 64 bit signed
> host type and getting a sign-extension, when the semantics of
> those particular syscalls require unsigned values. But conversely
> the change probably means that places which wanted the
> sign-extension are no longer getting it.
> 
> If we were writing this code from scratch then there's probably
> a good argument for making the arg* be the unsigned type
> rather than signed. Unfortunately at this point it's basically
> impossible to change over, because we'd have to audit every
> use of them in a 10,000 line file to determine whether we needed
> to put a cast back in to get sign extension or not. I'd rather we
> just fixed the places that don't want sign-extension, because
> presumably we at least have examples of failing guest programs
> we can use to tell us what the problematic syscalls are...

You snipped the part of my message where I was clearly *not* advocating
to apply Alex' type change but suggested to annotate arg uses with the
actual type, so that intermediate casts can be inserted. If we were to
agree on some system, that could be done incrementally.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg