[Qemu-devel] [PATCH] linux-user: Remove type casts to union type

[Qemu-devel] [PATCH] linux-user: Remove type casts to union type

[Stefan Weil]

Casting to a union type is a gcc (and clang) extension. Other compilers
might not support it. This is not a problem today, but the type casts
can be removed easily. Smatch now no longer complains like before:

linux-user/syscall.c:3190:18: warning: cast to non-scalar
linux-user/syscall.c:7348:44: warning: cast to non-scalar

Cc: Riku Voipio <riku.voipio@iki.fi>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 linux-user/syscall.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 852308e..ec137db 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2663,8 +2663,9 @@ static inline abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
 }
 
 static inline abi_long do_semctl(int semid, int semnum, int cmd,
-                                 union target_semun target_su)
+                                 abi_ulong target_arg)
 {
+    union target_semun target_su;
     union semun arg;
     struct semid_ds dsarg;
     unsigned short *array = NULL;
@@ -2673,6 +2674,8 @@ static inline abi_long do_semctl(int semid, int semnum, int cmd,
     abi_long err;
     cmd &= 0xff;
 
+    target_su.buf = target_arg;
+
     switch( cmd ) {
 	case GETVAL:
 	case SETVAL:
@@ -3186,8 +3189,7 @@ static abi_long do_ipc(unsigned int call, abi_long first,
          * ptr argument. */
         abi_ulong atptr;
         get_user_ual(atptr, ptr);
-        ret = do_semctl(first, second, third,
-                (union target_semun) atptr);
+        ret = do_semctl(first, second, third, atptr);
         break;
     }
 
@@ -7345,7 +7347,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_semctl
     case TARGET_NR_semctl:
-        ret = do_semctl(arg1, arg2, arg3, (union target_semun)(abi_ulong)arg4);
+        ret = do_semctl(arg1, arg2, arg3, arg4);
         break;
 #endif
 #ifdef TARGET_NR_msgctl
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] linux-user: Remove type casts to union type

[Michael Tokarev]

08.02.2015 17:40, Stefan Weil wrote:
> Casting to a union type is a gcc (and clang) extension. Other compilers
> might not support it. This is not a problem today, but the type casts
> can be removed easily. Smatch now no longer complains like before:
> 
> linux-user/syscall.c:3190:18: warning: cast to non-scalar
> linux-user/syscall.c:7348:44: warning: cast to non-scalar
> 
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  linux-user/syscall.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 852308e..ec137db 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2663,8 +2663,9 @@ static inline abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
>  }
>  
>  static inline abi_long do_semctl(int semid, int semnum, int cmd,
> -                                 union target_semun target_su)
> +                                 abi_ulong target_arg)
>  {
> +    union target_semun target_su;
>      union semun arg;
>      struct semid_ds dsarg;
>      unsigned short *array = NULL;
> @@ -2673,6 +2674,8 @@ static inline abi_long do_semctl(int semid, int semnum, int cmd,
>      abi_long err;
>      cmd &= 0xff;
>  
> +    target_su.buf = target_arg;


Can we use c99 initializers at declaration, something like

   union target_semun target_su = { .buf = target_arg }

?  Or is it also some gcc/clang extension? :)

But I'd like to hear from Riku at least...

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] linux-user: Remove type casts to union type

[Stefan Weil]

Am 10.02.2015 um 20:51 schrieb Michael Tokarev:
> 08.02.2015 17:40, Stefan Weil wrote:
>> Casting to a union type is a gcc (and clang) extension. Other compilers
>> might not support it. This is not a problem today, but the type casts
>> can be removed easily. Smatch now no longer complains like before:
>>
>> linux-user/syscall.c:3190:18: warning: cast to non-scalar
>> linux-user/syscall.c:7348:44: warning: cast to non-scalar
>>
>> Cc: Riku Voipio <riku.voipio@iki.fi>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  linux-user/syscall.c |   10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 852308e..ec137db 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -2663,8 +2663,9 @@ static inline abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
>>  }
>>  
>>  static inline abi_long do_semctl(int semid, int semnum, int cmd,
>> -                                 union target_semun target_su)
>> +                                 abi_ulong target_arg)
>>  {
>> +    union target_semun target_su;
>>      union semun arg;
>>      struct semid_ds dsarg;
>>      unsigned short *array = NULL;
>> @@ -2673,6 +2674,8 @@ static inline abi_long do_semctl(int semid, int semnum, int cmd,
>>      abi_long err;
>>      cmd &= 0xff;
>>  
>> +    target_su.buf = target_arg;
> 
> 
> Can we use c99 initializers at declaration, something like
> 
>    union target_semun target_su = { .buf = target_arg }
> 
> ?  Or is it also some gcc/clang extension? :)
> 
> But I'd like to hear from Riku at least...
> 
> Thanks,
> 
> /mjt
> 

Ping. This patch is still missing.

Hello Michael,

using a c99 initializer is a good idea. We already have lots
of them, it's not a gcc extension but C standard.

Should I send an updated patch, or do you want to modify
the code before applying it?

Regards,
Stefan

Re: [Qemu-devel] [PATCH] linux-user: Remove type casts to union type

[Michael Tokarev]

08.02.2015 17:40, Stefan Weil wrote:
> Casting to a union type is a gcc (and clang) extension. Other compilers
> might not support it. This is not a problem today, but the type casts
> can be removed easily. Smatch now no longer complains like before:

I've applied this patch with the following change:

>  linux-user/syscall.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 852308e..ec137db 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2663,8 +2663,9 @@ static inline abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
>  }
>  
>  static inline abi_long do_semctl(int semid, int semnum, int cmd,
> -                                 union target_semun target_su)
> +                                 abi_ulong target_arg)
>  {
> +    union target_semun target_su;

       union target_semun target_su = { .buf = target_arg };

>      union semun arg;
>      struct semid_ds dsarg;
>      unsigned short *array = NULL;
> @@ -2673,6 +2674,8 @@ static inline abi_long do_semctl(int semid, int semnum, int cmd,
>      abi_long err;
>      cmd &= 0xff;
>  
> +    target_su.buf = target_arg;

and dropping this hunk.

Thank you!

/mjt