Re: [PATCH v2 2/2] linux-user: fix print_syscall_err() when syscall returned value is negative

[Laurent Vivier]

Le 08/07/2020 à 17:52, Richard Henderson a écrit :
> On 7/8/20 8:24 AM, Laurent Vivier wrote:
>> -static void
>> +static bool
>>  print_syscall_err(abi_long ret)
>>  {
>> -    const char *errstr = NULL;
>> +    const char *errstr;
>>  
>>      qemu_log(" = ");
>>      if (ret < 0) {
> 
> This should be a target-specific test.
> 
> E.g. on most asm-generic I'm pretty sure this should be
> 
>     if ((abi_ulong)ret > -(abi_ulong)512)

I think the test in target_strerror() gives the same result:

    if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
        return NULL;
    }

and it also ensures we don't overflow when we will access
target_to_host_errno_table[].

It's why we rely on errstr to know if the errno is valid or not
(we might also remove the "if (ret < 0)" in print_syscall_err).

> whereas for Alpha it should be
> 
>     /*
>      * Syscall writes 0 to V0 to bypass error check, similar
>      * to how this is handled internal to Linux kernel.
>      */
>     if (ret < 0 && env->ir[IR_V0] != 0)

We don't have access to "env" in strace.c.

it's an improvement regarding the code that has been modified.
If we want it I think it should be added in a separate patch.

Thanks,
Laurent