Patchwork [v2] linux-user: add ppoll syscall support

login
register
mail settings
Submitter Mike Frysinger
Date Jan. 23, 2011, 11:32 p.m.
Message ID <1295825542-12550-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/80081/
State New
Headers show

Comments

Mike Frysinger - Jan. 23, 2011, 11:32 p.m.
Some architectures (like Blackfin) only implement ppoll (and skip poll).
So add support for it using existing poll code.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- call host ppoll() directly

 linux-user/syscall.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)
Peter Maydell - Jan. 24, 2011, 12:35 a.m.
On 23 January 2011 23:32, Mike Frysinger <vapier@gentoo.org> 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

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6116ab5..690ee44 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6230,8 +6230,13 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_select(arg1, arg2, arg3, arg4, arg5);
         break;
 #endif
-#ifdef TARGET_NR_poll
+#if defined(TARGET_NR_poll) || defined(TARGET_NR_ppoll)
+# ifdef TARGET_NR_poll
     case TARGET_NR_poll:
+# endif
+# ifdef TARGET_NR_ppoll
+    case TARGET_NR_ppoll:
+# endif
         {
             struct target_pollfd *target_pfd;
             unsigned int nfds = arg2;
@@ -6242,12 +6247,31 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             target_pfd = lock_user(VERIFY_WRITE, arg1, sizeof(struct target_pollfd) * nfds, 1);
             if (!target_pfd)
                 goto efault;
+
             pfd = alloca(sizeof(struct pollfd) * nfds);
             for(i = 0; i < nfds; i++) {
                 pfd[i].fd = tswap32(target_pfd[i].fd);
                 pfd[i].events = tswap16(target_pfd[i].events);
             }
-            ret = get_errno(poll(pfd, nfds, timeout));
+
+# ifdef TARGET_NR_ppoll
+            if (num == TARGET_NR_ppoll) {
+                struct timespec _timeout_ts, *timeout_ts = &_timeout_ts;
+                abi_ulong mask = arg4;
+                sigset_t sigmask;
+
+                if (arg3)
+                    target_to_host_timespec(timeout_ts, arg3);
+                else
+                    timeout_ts = NULL;
+
+                target_to_host_old_sigset(&sigmask, &mask);
+
+                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);