Date: Tue, 04 Mar 2008 18:35:29 +0100 From: Alexander Leidinger <Alexander@Leidinger.net> To: rdivacky@freebsd.org Cc: emulation@freebsd.org Subject: epoll review continued Message-ID: <20080304183529.kpaf3f76gcg4g0cs@webmail.leidinger.net>
next in thread | raw e-mail | index | archive | help
> @@ -57,7 +57,9 @@
>
> =09if (args->size <=3D 0)
> =09=09return (EINVAL);
> -=09/* args->size is unused. Linux ignores it as well. */
> +=09/* +=09 * args->size is unused. Linux just tests it +=09 * and then =
=20
> forgets it as well. */
My proposal ("Submitted by: netchild" in the log):
/*
* The size is documented to be only a hint. We don't need
* it with kevent. The epoll_create man-page tells that the
* size has to be not-negative, but the linux kernel test
* for a value of at least 1, so for error compatibility we
* do the same.
*/
The above comment should be before the if, not after.
> =09return (kqueue(td, &k_args));
> }
> @@ -140,7 +142,9 @@
>
> /*
> * Copyin callback used by kevent. This copies already
> - * converted filters to the kevent internal memory.
> + * converted filters from kernel memory to the kevent + * internal =20
> kernel memory. Hence the memcpy instead of
> + * copyin.
> */
Thanks.
> static int
> linux_kev_copyin(void *arg, struct kevent *kevp, int count)
> @@ -193,7 +197,8 @@
> =09=09break;
> =09case LINUX_EPOLL_CTL_MOD:
> =09=09=09/* TODO: DELETE && ADD maybe? */
> -=09=09=09return (EINVAL);
> +=09=09=09printf("linux_epoll_ctl: CTL_MOD not yet >implemented.\n");
> +=09=09=09return (ENOSYS);
ENOSYS is "syscall not implemented". But we implement the syscall. =20
When glibc tests this syscall and gets back a ENOSYS, it may switch to =20
a different way of monitoring files. I think the EINVAL was better, as =20
it will result in an application error and notify us that we need to =20
take care about this part of the syscall.
> =09=09break;
> =09case LINUX_EPOLL_CTL_DEL:
> =09=09=09kev.flags =3D EV_DELETE | EV_DISABLE;
> @@ -241,6 +246,9 @@
>
> =09error =3D kern_kevent(td, args->epfd, 0, args->maxevents, &k_ops, &ts);
>
> -=09/* translation? */
> +=09/* +=09 * kern_keven might return ENOMEM which is not expected from =
=20
> epoll_wait.
> +=09 * Maybe we should translate that but I don't think it matters at all.
> +=09 */
> =09return (error);
> }
It is not even ENOMEM. After a quick look I think it should be like:
- ESRCH, ENOENT should map to EINVAL
- EACCESS should map to EFAULT or EINVAL (don't really know)
- ENOMEM should map to... ?
You also need to deliver EINVAL, if maxevents is <=3D 0 (according to =20
the man page of epoll_wait).
Bye,
Alexander.
--=20
If you sow your wild oats, hope for a crop failure.
http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID =3D B0063FE7
http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID =3D 72077137
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080304183529.kpaf3f76gcg4g0cs>