linux-user/syscall.c : Minor cleanups of timer_create handling.

* 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(-)

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

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

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

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

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