Re: [Qemu-devel] [PATCH v2] linux-user: add ppoll syscall support

[Peter Maydell]

On 23 January 2011 23:32, Mike Frysinger <address@hidden> wrote:
> +                if (arg3)
> +                    target_to_host_timespec(timeout_ts, arg3);
> +                else
> +                    timeout_ts = NULL;

Coding style mandates braces here. Also, target_to_host_timespec()
can return non-zero if the user handed us a bad pointer, which
you need to handle. (No, none of the other users of the
function do this; yes, I think they're all broken :-))

> +                target_to_host_old_sigset(&sigmask, &mask);

Are you sure this is right?
http://lxr.linux.no/#linux+v2.6.37/fs/select.c#L950
suggests the syscall takes a new sigset, not an old one.

You also need to lock_user() the memory for the sigset.
(target_to_host_timespec() does lock_user/unlock_user for
you but the target_to_host_*sigset() don't).

> +                ret = get_errno(ppoll(pfd, nfds, timeout_ts, &sigmask));
> +            } else
> +# endif
> +                ret = get_errno(poll(pfd, nfds, timeout));
> +
>             if (!is_error(ret)) {
>                 for(i = 0; i < nfds; i++) {
>                     target_pfd[i].revents = tswap16(pfd[i].revents);

The ppoll() manpage says
"The Linux ppoll() system call modifies its timeout argument."

Your implementation doesn't do this. Also, this means you
really do need to call the host ppoll syscall directly, because
glibc deliberately hides this behaviour and would prevent
us from implementing it.

thanks
-- PMM