In article <20050710221533.GE28480@gallia.cubidou.net>,
Quentin Garnier  <cube@cubidou.net> wrote:
>-=-=-=-=-=-
>-=-=-=-=-=-
>
>Hi folks,
>
>On my path to a complete netbsd32 emulation, I could see that some
>syscalls in the compat_netbsd32 code were copied and adapted from the
>native counterpart.
>
>The problem is that those syscalls are very hard to maintain.  I fixed
>netbsd32_wait4() this morning, and this afternoon, while trying to
>implement rasctl(2), I discovered that netbsd32_execve() was broken in
>a similar way:  some changes done to the native syscall have not been
>reported to the compat_netbsd32 version.
>
>The reason why there is a need for a slightly different version is
>because those syscalls manipulate data in userspace that have different
>size depending on the emulation.  Therefore you cannot always use
>copyin()/copyout().
>
>To properly fix netbsd32_wait4(), the only thing that would be needed is
>a slighly smarter copyin()/copyout() that would check if the supposedly
>user address is actually a kernel address or not, and in the former case
>only do a memcpy().  I don't know if it easy to do that, even if it ends
>as a set of MD implementations.  I do know that it wouldn't be only
>useful for compat_netbsd32, though.
>
>However, a smarter copyin() is not enough when it comes to arrays of
>structs of a size different from the native version.  This is the case
>of execve(2) (and others, I know at least kevent(2)).
>
>netbsd32_execve() was missing the calls related to RAS support, which
>showed that however close it was to the native sys_execve(), it had
>drifted apart from it.
>
>The solution I propose for such syscalls that access an array in user
>space is to use a helper function that takes an additional argument
>which is a pointer to a function that will do the copyin() (or a
>copyout(), in execve(2)'s case it's only a copyin() though), and the
>translation in the compat_netbsd32 case.
>
>That way the actual syscall is only slowed down by a few function calls,
>and remain readable.
>
>See for yourself with the attached patch for execve(2).
>
>There might be smarter ways of doing this, I'm open to suggestions.  But
>keep in mind that we want to avoid as much as possible duplicating code,
>but not at the expense of the native syscall's speed.
>
>The other solution is to teach people to think of compat_netbsd32
>whenever they touch a syscall, but this has obviously failed, and is
>doomed to fail anyway.
>
>Comments?

I like it a lot because it simplifies the code and improves maintainability
because we don't need to maintain 2 copies.

1. I would have used size_t instead of u_int for the count.
2. Calling the function ptr should be (*foo)(args) not foo(args).
3. There is an extra space before paren in the line ... NULL )

christos