[Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Erik de Castro Lopo]

* Add missing unlock of user struct.
* Remove unneeded pointer variable.

Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
---
 linux-user/syscall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..7d8f54a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9412,7 +9412,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */
 
-        struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL;
+        struct sigevent host_sevp = { {0}, };
         struct target_sigevent *ptarget_sevp;
         struct target_timer_t *ptarget_timer;
 
@@ -9432,10 +9432,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
                 host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
 
-                phost_sevp = &host_sevp;
+                unlock_user_struct(ptarget_sevp, arg2, 0);
             }
 
-            ret = get_errno(timer_create(clkid, phost_sevp, phtimer));
+            ret = get_errno(timer_create(clkid, &host_sevp, phtimer));
             if (ret) {
                 phtimer = NULL;
             } else {
-- 
2.0.1

Re: [Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Peter Maydell]

On 2 August 2014 15:10, Erik de Castro Lopo <erikd@mega-nerd.com> wrote:
> * Add missing unlock of user struct.
> * Remove unneeded pointer variable.
>
> Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
> ---
>  linux-user/syscall.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a50229d..7d8f54a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9412,7 +9412,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      {
>          /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */
>
> -        struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL;
> +        struct sigevent host_sevp = { {0}, };
>          struct target_sigevent *ptarget_sevp;
>          struct target_timer_t *ptarget_timer;
>
> @@ -9432,10 +9432,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
>                  host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
>
> -                phost_sevp = &host_sevp;
> +                unlock_user_struct(ptarget_sevp, arg2, 0);
>              }
>
> -            ret = get_errno(timer_create(clkid, phost_sevp, phtimer));
> +            ret = get_errno(timer_create(clkid, &host_sevp, phtimer));
>              if (ret) {
>                  phtimer = NULL;
>              } else {

Doesn't this turn a timer_create(clkid, NULL, phtimer) into a
timer_create(clkid, something-not-NULL, phtimer) ? That
doesn't seem right to me (and the code you've deleted here
is the common idiom in syscall.c for handling those "arg
is pointer-to-struct-or-NULL" cases).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Erik de Castro Lopo]

Peter Maydell wrote:

> Doesn't this turn a timer_create(clkid, NULL, phtimer) into a
> timer_create(clkid, something-not-NULL, phtimer) ? That
> doesn't seem right to me (and the code you've deleted here
> is the common idiom in syscall.c for handling those "arg
> is pointer-to-struct-or-NULL" cases).

You're right. Thanks. I will amend this.

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Re: [Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Peter Maydell]

On 2 August 2014 23:48, Erik de Castro Lopo <mle+tools@mega-nerd.com> wrote:
> Peter Maydell wrote:
>
>> Doesn't this turn a timer_create(clkid, NULL, phtimer) into a
>> timer_create(clkid, something-not-NULL, phtimer) ? That
>> doesn't seem right to me (and the code you've deleted here
>> is the common idiom in syscall.c for handling those "arg
>> is pointer-to-struct-or-NULL" cases).
>
> You're right. Thanks. I will amend this.

Amend it to what? The current code looks fine to me,
so I'm not sure what bug you're trying to fix here.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Erik de Castro Lopo]

Peter Maydell wrote:

> Amend it to what? The current code looks fine to me,
> so I'm not sure what bug you're trying to fix here.

There is still a missing call to unlock_user_struct() inside
the "if (arg2)" block. Is that not worth fixing?

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Re: [Qemu-devel] [PATCH] linux-user/syscall.c : Minor cleanups of timer_create handling.

[Peter Maydell]

On 3 August 2014 00:21, Erik de Castro Lopo <mle+tools@mega-nerd.com> wrote:
> Peter Maydell wrote:
>
>> Amend it to what? The current code looks fine to me,
>> so I'm not sure what bug you're trying to fix here.
>
> There is still a missing call to unlock_user_struct() inside
> the "if (arg2)" block. Is that not worth fixing?

Oops, yes; sorry, I missed that part of your commit message.

-- PMM

[Qemu-devel] [PATCH] linux-user: Add missing unlock_user_struct to timer_create.

[Erik de Castro Lopo]

Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
---
 linux-user/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..5f22b37 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9432,6 +9432,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
                 host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
 
+                unlock_user_struct(ptarget_sevp, arg2, 0);
                 phost_sevp = &host_sevp;
             }
 
-- 
2.0.1

Re: [Qemu-devel] [PATCH] linux-user: Add missing unlock_user_struct to timer_create.

[Peter Maydell]

On 3 August 2014 00:45, Erik de Castro Lopo <erikd@mega-nerd.com> wrote:
> Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
> ---
>  linux-user/syscall.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a50229d..5f22b37 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9432,6 +9432,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
>                  host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
>
> +                unlock_user_struct(ptarget_sevp, arg2, 0);
>                  phost_sevp = &host_sevp;
>              }
>
> --

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Add missing unlock_user_struct to timer_create.

[Riku Voipio]

On Sun, Aug 03, 2014 at 09:45:38AM +1000, Erik de Castro Lopo wrote:
> Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>

Thanks,

applied to linux-user tree.

> ---
>  linux-user/syscall.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a50229d..5f22b37 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9432,6 +9432,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
>                  host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
>  
> +                unlock_user_struct(ptarget_sevp, arg2, 0);
>                  phost_sevp = &host_sevp;
>              }
>  
> -- 
> 2.0.1
>