On 21.11.2011, at 11:47, Peter Maydell wrote:
> On 21 November 2011 10:40, Alexander Graf <ag...@suse.de> wrote:
>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>> index 90027a1..e30b005 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -1515,14 +1515,19 @@ void
>> print_syscall_ret(int num, abi_long ret)
>> {
>> int i;
>> + char *errstr = NULL;
>>
>> for(i=0;i<nsyscalls;i++)
>> if( scnames[i].nr == num ) {
>> if( scnames[i].result != NULL ) {
>> scnames[i].result(&scnames[i],ret);
>> } else {
>> - if( ret < 0 ) {
>> - gemu_log(" = -1 errno=" TARGET_ABI_FMT_ld " (%s)\n",
>> -ret, target_strerror(-ret));
>> + if (ret < 0) {
>> + errstr = target_strerror(-ret);
>> + }
>> + if (errstr) {
>> + gemu_log(" = -1 errno=" TARGET_ABI_FMT_ld " (%s)\n",
>> + -ret, errstr);
>
> Should this really be printing -1 all the time when ret isn't -1 ?
In linux-user, the syscall emulation functions return -errno which later gets
translated to -1 with errno=-errno. That's why the code looks so weird. So yes,
it's correct.
>
>> } else {
>> gemu_log(" = " TARGET_ABI_FMT_ld "\n", ret);
>> }
>
> print_syscall_ret_addr() also needs to handle NULL
> returns from target_strerror().
I don't think they can ever happen, since we're sure to use valid errnos there.
But I can add a check.
>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index f227097..c59d00e 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -731,6 +731,9 @@ static inline int is_error(abi_long ret)
>>
>> char *target_strerror(int err)
>> {
>> + if ((err >= ERRNO_TABLE_SIZE) && (err >= 0)) {
>> + return NULL;
>> + }
>> return strerror(target_to_host_errno(err));
>
> shouldn't that be ((err >= ERRNO_TABLE_SIZE) || (err < 0)) ?
Yes.
Alex
