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

[Pierre Muller]

 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);