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