[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 1/1] linux-user: Support futex_time64
From: |
Laurent Vivier |
Subject: |
Re: [PULL 1/1] linux-user: Support futex_time64 |
Date: |
Tue, 3 Nov 2020 08:28:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
Le 02/11/2020 à 19:15, Peter Maydell a écrit :
> On Mon, 30 Mar 2020 at 11:31, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> From: Alistair Francis <alistair.francis@wdc.com>
>>
>> Add support for host and target futex_time64. If futex_time64 exists on
>> the host we try that first before falling back to the standard futex
>> syscall.
>
> Hi; I dunno why Coverity's only just noticed this, but in
> CID 1432339 it points out:
>
>> +#if defined(TARGET_NR_futex_time64)
>> +static int do_futex_time64(target_ulong uaddr, int op, int val,
>> target_ulong timeout,
>> + target_ulong uaddr2, int val3)
>> +{
>> + struct timespec ts, *pts;
>> + int base_op;
>> +
>> + /* ??? We assume FUTEX_* constants are the same on both host
>> + and target. */
>> +#ifdef FUTEX_CMD_MASK
>> + base_op = op & FUTEX_CMD_MASK;
>> +#else
>> + base_op = op;
>> +#endif
>> + switch (base_op) {
>> + case FUTEX_WAIT:
>> + case FUTEX_WAIT_BITSET:
>> + if (timeout) {
>> + pts = &ts;
>> + target_to_host_timespec64(pts, timeout);
>
> ...that here we call target_to_host_timespec64(), which can
> fail with -TARGET_EFAULT, but (unlike all the other times we call
> the function) we aren't checking its return value.
> Is there missing error handling code here ?
>
I think the code is like that because this is a cut&paste of function
do_futex() witl "s/timespec/timespec64/".
And yes I think we should check for the return value.
I'm going to fix that.
Thanks,
Laurent