diff mbox series

[v3,15/17] linux-user/strace: Fix print_syscall_err

Message ID 20220316055840.727571-16-richard.henderson@linaro.org
State Superseded
Headers show
Series target/m68k: Conditional traps + trap cleanup | expand

Commit Message

Richard Henderson March 16, 2022, 5:58 a.m. UTC
Errors are not all negative numbers, but only the top 4k.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pierre Muller March 17, 2022, 10:43 a.m. UTC | #1
Hello Richard,

   being mainly a Pascal programmer,
I always jump when I see such constructs:
   You change the type of the print_syscall_err parameter to abi_ulong,
with is an unsigned type as the name suggests,
and after you compare it to a signed constant -4096.

   This would never work in Pascal, but this is of course C code.

   Nevertheless, I tried:

muller@gcc140:~/pas/check$ cat ./test-unsigned-int.c
#include <stdio.h>

int test_is_error (unsigned int u)
{
   return (u > -4096);
}

int main ()
{
   unsigned int v;
   int error_count = 0;
   v = (unsigned int) - 3000;
   if (test_is_error(v) != 0)
     printf("v=-3000 accepted\n");
   else
     error_count++;
   v = (unsigned int) - 5000;
   if (test_is_error(v) == 0)
     printf("v=-5000 rejected\n");
   else
     error_count++;
   v = (unsigned int) 0xfffffff0;
   if (test_is_error(v) != 0)
     printf("v=%d accepted\n",v);
   else
     error_count++;
   if (test_is_error(0xffffff00) != 0)
     printf("v=%d accepted\n",0xffffff00);
   else
     error_count++;
   return error_count;
}


muller@gcc140:~/pas/check$ gcc  -W -o test-unsigned-int test-unsigned-int.c
test-unsigned-int.c: In function ‘test_is_error’:
test-unsigned-int.c:5:13: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
     5 |   return (u > -4096);
       |             ^

The results are OK (both for 32 and 64 bit executables),
but is there a cleaner way, which does not generate a warning, to write this?

Adding an explicit typecast to -4096 seems sufficient to silence that warning.


Pierre Muller


Le 16/03/2022 à 06:58, Richard Henderson a écrit :
> Errors are not all negative numbers, but only the top 4k.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 2cdbf030ba..f235118fb6 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -684,12 +684,12 @@ print_ipc(void *cpu_env, const struct syscallname *name,
>    */
>   
>   static bool
> -print_syscall_err(abi_long ret)
> +print_syscall_err(abi_ulong ret)
>   {
>       const char *errstr;
>   
>       qemu_log(" = ");
> -    if (ret < 0) {
> +    if (ret > -4096) {
>           errstr = target_strerror(-ret);
>           if (errstr) {
>               qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
diff mbox series

Patch

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 2cdbf030ba..f235118fb6 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -684,12 +684,12 @@  print_ipc(void *cpu_env, const struct syscallname *name,
  */
 
 static bool
-print_syscall_err(abi_long ret)
+print_syscall_err(abi_ulong ret)
 {
     const char *errstr;
 
     qemu_log(" = ");
-    if (ret < 0) {
+    if (ret > -4096) {
         errstr = target_strerror(-ret);
         if (errstr) {
             qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);