Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 8 Nov 2014 12:28:11 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Dmitry Chagin <dchagin@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   Re: svn commit: r274257 - in user/dchagin/lemul/sys: compat/freebsd32 kern
Message-ID:  <20141108102811.GU53947@kib.kiev.ua>
In-Reply-To: <201411072312.sA7NC86v017392@svn.freebsd.org>
References:  <201411072312.sA7NC86v017392@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 07, 2014 at 11:12:08PM +0000, Dmitry Chagin wrote:
> Author: dchagin
> Date: Fri Nov  7 23:12:07 2014
> New Revision: 274257
> URL: https://svnweb.freebsd.org/changeset/base/274257
> 
> Log:
>   Add native ppoll system call.
> 
> Modified:
>   user/dchagin/lemul/sys/compat/freebsd32/freebsd32_misc.c
>   user/dchagin/lemul/sys/compat/freebsd32/syscalls.master
>   user/dchagin/lemul/sys/kern/sys_generic.c
>   user/dchagin/lemul/sys/kern/syscalls.master
> 
> Modified: user/dchagin/lemul/sys/compat/freebsd32/freebsd32_misc.c
> ==============================================================================
> --- user/dchagin/lemul/sys/compat/freebsd32/freebsd32_misc.c	Fri Nov  7 22:52:02 2014	(r274256)
> +++ user/dchagin/lemul/sys/compat/freebsd32/freebsd32_misc.c	Fri Nov  7 23:12:07 2014	(r274257)
> @@ -3017,3 +3017,31 @@ freebsd32_fcntl(struct thread *td, struc
>  	}
>  	return (kern_fcntl_freebsd(td, uap->fd, uap->cmd, tmp));
>  }
> +
> +int
> +freebsd32_ppoll(struct thread *td, struct freebsd32_ppoll_args *uap)
> +{
> +	struct timespec32 ts32;
> +	struct timespec ts, *tsp;
> +	sigset_t set, *ssp;
> +	int error;
> +
> +	if (uap->ts != NULL) {
> +		error = copyin(uap->ts, &ts32, sizeof(ts32));
> +		if (error != 0)
> +			return (error);
> +		CP(ts32, ts, tv_sec);
> +		CP(ts32, ts, tv_nsec);
> +		tsp = &ts;
> +	} else
> +		tsp = NULL;
> +	if (uap->set != NULL) {
> +		error = copyin(uap->set, &set, sizeof(set));
> +		if (error != 0)
> +			return (error);
> +		ssp = &set;
> +	} else
> +		ssp = NULL;
> +
> +	return (kern_ppoll(td, uap->fds, uap->nfds, tsp, ssp));
> +}
> 
> Modified: user/dchagin/lemul/sys/compat/freebsd32/syscalls.master
> ==============================================================================
> --- user/dchagin/lemul/sys/compat/freebsd32/syscalls.master	Fri Nov  7 22:52:02 2014	(r274256)
> +++ user/dchagin/lemul/sys/compat/freebsd32/syscalls.master	Fri Nov  7 23:12:07 2014	(r274257)
> @@ -1066,3 +1066,6 @@
>  				    uint32_t id1, uint32_t id2, int com, \
>  				    void *data); }
>  #endif
> +545	AUE_POLL	STD	{ int freebsd32_ppoll(struct pollfd *fds, \
> +				    u_int nfds, const struct timespec32 *ts, \
> +				    const sigset_t *set); }
> 
> Modified: user/dchagin/lemul/sys/kern/sys_generic.c
> ==============================================================================
> --- user/dchagin/lemul/sys/kern/sys_generic.c	Fri Nov  7 22:52:02 2014	(r274256)
> +++ user/dchagin/lemul/sys/kern/sys_generic.c	Fri Nov  7 23:12:07 2014	(r274257)
> @@ -1377,6 +1377,41 @@ out:
>  	return (error);
>  }
>  
> +#ifndef _SYS_SYSPROTO_H_
> +struct ppoll_args {
> +	struct pollfd *fds;
> +	u_int	nfds;
> +	struct timespec	*ts;
> +	sigset_ *set;
This is spelled sigset_t.  In fact, I do not think we should keep maintain
the unused manually copied syscall arg structs definitions.  At least,
I never added them for new syscalls.

> +};
> +#endif
> +int
> +sys_ppoll(td, uap)
> +	struct thread *td;
> +	struct ppoll_args *uap;
Please use C89 function definitions for new code.

> +{
> +	struct timespec ts, *tsp;
> +	sigset_t set, *ssp;
> +	int error;
> +
> +	if (uap->ts != NULL) {
> +		error = copyin(uap->ts, &ts, sizeof(ts));
> +		if (error)
> +			return (error);
> +		tsp = &ts;
> +	} else
> +		tsp = NULL;
> +	if (uap->set != NULL) {
> +		error = copyin(uap->set, &set, sizeof(set));
> +		if (error)
> +			return (error);
> +		ssp = &set;
> +	} else
> +		ssp = NULL;
> +
> +	return (kern_ppoll(td, uap->fds, uap->nfds, tsp, ssp));
> +}
> +
>  int
>  kern_ppoll(struct thread *td, struct pollfd *fds, u_int nfds,
>      struct timespec *tsp, sigset_t *uset)

One more note about kern_ppoll().  As I see, tsp and uset are supposed
to be in the KVA, while fds points to UVA.  This is consistent with
kern_select(), but is surprising for somebody who reads the code.
Please add comments explaining the conventions.

Also, I think ppoll(2) should be extracted from the branch and committed
on its own.

What about man page ?  Or update of poll(2).



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141108102811.GU53947>