svn commit: r274257 - in user/dchagin/lemul/sys: compat/freebsd32 kern

[Baptiste Daroussin]

On Sat, Nov 08, 2014 at 05:16:40PM +0300, Chagin Dmitry wrote:
> On Sat, Nov 08, 2014 at 12:28:11PM +0200, Konstantin Belousov wrote:
> > 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.
> > 
> 
> yeah, thank you! i'll fix soon.
> 
> 
> > > +};
> > > +#endif
> > > +int
> > > +sys_ppoll(td, uap)
> > > +	struct thread *td;
> > > +	struct ppoll_args *uap;
> > Please use C89 function definitions for new code.
> > 
> 
> sure
> 
> > > +{
> > > +	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.
> 
> ok
> > 
> > Also, I think ppoll(2) should be extracted from the branch and committed
> > on its own.
> 
> agree
> > 
> > What about man page ?  Or update of poll(2).
> 
> oh.. I'll try, it's not easy for me
> 
If you need review/help on this you can use phabricator for asking some people
to review there is a '#manpages' group with people willing to help on the
manpage side of the code :)

Best regards,
Bapt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-user/attachments/20141108/8fb0f2ae/attachment.sig>