Le 02/11/2020 à 19:15, Peter Maydell a écrit : > On Mon, 30 Mar 2020 at 11:31, Laurent Vivier <laur...@vivier.eu> wrote: >> >> From: Alistair Francis <alistair.fran...@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