svn commit: r236026 - in head/sys: amd64/linux32
compat/freebsd32 kern
Konstantin Belousov
kostikbel at gmail.com
Sat May 26 17:13:52 UTC 2012
On Sat, May 26, 2012 at 10:21:25PM +1000, Bruce Evans wrote:
> On Fri, 25 May 2012, Ed Schouten wrote:
>
> >Log:
> > Remove use of non-ISO-C integer types from system call tables.
> >
> > These files already use ISO-C-style integer types, so make them less
> > inconsistent by preferring the standard types.
>
> These should actually be Linux types l_foo_t. ISO-C-style integer types
> seem to have only been used for a couple of uintptr_t's, and these uses
> are more than just style bugs on amd64 since uintptr_t is for the host
> (64 bits on amd64) while the target uintptr_t is only 32 bits. There
> are also a few misuses of the abominable caddr_t instead of l_caddr_t,
> so syscalls that don't even take a caddr_t. Otherwise, Linux types
> are used a lot to avoid size mismatches.
>
> >Modified: head/sys/amd64/linux32/syscalls.master
> >==============================================================================
> >--- head/sys/amd64/linux32/syscalls.master Fri May 25 21:12:24 2012
> >(r236025)
> >+++ head/sys/amd64/linux32/syscalls.master Fri May 25 21:50:48 2012
> >(r236026)
> >@@ -54,8 +54,8 @@
> > l_int mode); }
> >9 AUE_LINK STD { int linux_link(char *path, char *to); }
> >10 AUE_UNLINK STD { int linux_unlink(char *path); }
> >-11 AUE_EXECVE STD { int linux_execve(char *path, u_int32_t
> >*argp, \
> >- u_int32_t *envp); }
> >+11 AUE_EXECVE STD { int linux_execve(char *path, uint32_t
> >*argp, \
> >+ uint32_t *envp); }
>
> argp and envp aren't uintany_t * in either Linux or FreeBSD. They start as
> "char * const *". There is no Linux type for an indirect "char *", and one
> was hacked up here by pretending that "char *" is u_int32_t (it is actually
> just 32 bits). Using l_caddr_t seems to be the best hack available (since
> by abusing l_caddr_t, we know that it is actually char *).
>
> The `const' in the type for argp and envp is further from being handled
> correctly. Most or all syscall.master's just type-pun it away. Similarly
> for "const char *path".
>
> All the non-indirect "char *"s for pathnames and other things seem to be
> completely wrong on amd64 too. These pointers start as 32 bits, and it
> takes more than a bad type pun to turn then into kernel 64-bit pointers.
> The magic for this seems to be:
> - all args are converted to 64 bits (by zero-extension?) at a low level
The 'low level' AKA magic happens in several *_fetch_syscall_args()
functions. For both linux32 and freebsd32, the magic code automatically
zero-extends the arguments into 64bit entities. Linux passes args in
registers, while FreeBSD uses words on stack.
The types in the syscalls.master prototype should be in fact selected
to fit the in-kernel prototypes for the functions implementing the syscalls,
esp. for NOPROTO cases, and not to the low-level layout of the syscall
entry data.
> - the args struct for a pathname is
> [left padding]; char *; [right padding];
> Since the char * is misdeclared, the explicit padding is null, but the
> layout of the args struct is correct because the wrong arg type in it
> supplies equivalent padding (extra 32 bits on the right).
The arg struct layout is irrelevant, since fetch_syscall_args() functions
perform the needed translation from process ABI to kernel ABI.
I think that the padding could be completely eliminated for translated
ABI, but since it is easier to reuse makesyscalls.sh instead of creating
ABI-specific script, and since there is quite non-trivial count of NOPROTO
declarations that just match the native-ABI syscall handlers, it is better
not to start that.
> - the "char *" in the args struct is not actually a char *, and is unusable
> directly in the kernel. However, it is only used in copyin() and
> copyout(), where it becomes a user address and works correctly. (An
> older bug in this that the user address for copy*() is declared as
> "void *". "void *" means a kernel pointer. The type of a user
> address should be more like vm_offset_t, but even that needs logical
> translation for linux32).
It is char *, but in different address space. Linux-style type qualifiers
like __usermode would be appropriate there (remember far/near ?), but
we do not have static checkers that do understand the difference.
>
> The same mechanism presumably avoids problems when raw caddr_t is used
> instead of l_caddr_t, and when uintptr_t is used instead of l_uintptr_t.
>
> >12 AUE_CHDIR STD { int linux_chdir(char *path); }
> >13 AUE_NULL STD { int linux_time(l_time_t *tm); }
>
> Example of a correct use of a linux type. Again, the first-level pointer
> is handled by the above magic, but for the second level we need an l_foo_t
> to describe its size correctly.
>
> >14 AUE_MKNOD STD { int linux_mknod(char *path, l_int mode, \
>
> Broken except in the K&R case, but amd64 is too new to pretend to support
> K&R. Bug for bug compatible with mknod() and some other syscalls in
C std version is irrelevant there. Args fetch code promotes all arguments
to (u)int(32).
> kern/syscalls.master. mknod() takes an arg of type mode_t, not one of
> type int. l_mode_t exists so that the above can be declared correctly,
> and is already used for linux_chmod() and linux_chmodat(). OTOH, int for
> the mode arg is correct for open(), since open() is variadic at a level
> before the syscall, so its mode_t arg gets promoted to int. Similarly,
> if mknod() is compiled by a K&R compiler, or by a STDC compiler with no
> prototype in scope, then its mode_t arg gets promoted to int (but for
> the STDC case, the behaviour is then undefined once mknod() is reached).
> Normally this doesn't cause any problems, but it is easy to declare
> things correctly.
>
> >Modified: head/sys/compat/freebsd32/syscalls.master
> >==============================================================================
> >--- head/sys/compat/freebsd32/syscalls.master Fri May 25 21:12:24 2012
> >(r236025)
> >+++ head/sys/compat/freebsd32/syscalls.master Fri May 25 21:50:48 2012
> >(r236026)
> >@@ -104,9 +104,9 @@
> > int flags); }
> >28 AUE_SENDMSG STD { int freebsd32_sendmsg(int s, struct
> >msghdr32 *msg, \
> > int flags); }
> >-29 AUE_RECVFROM STD { int freebsd32_recvfrom(int s, u_int32_t
> >buf, \
> >- u_int32_t len, int flags, u_int32_t
> >from, \
> >- u_int32_t fromlenaddr); }
> >+29 AUE_RECVFROM STD { int freebsd32_recvfrom(int s, uint32_t
> >buf, \
> >+ uint32_t len, int flags, uint32_t from, \
> >+ uint32_t fromlenaddr); }
>
> Oops, I didn't looke at this file when I said that "ISO-C-style integer
> types seem to have only been used for a couple of uintptr_t's". This
> file is independent of Linux, so it can't use l_foo. It hard-codes 32
> instead, starting with the directory name. Still, all of the types in
> the above are fairly bogus and hard to understand:
> - "int" for `s' and `flags' depends on ints being 32 bits
I agree that this is the biggest type-pun in the whole compat32/linux32
components. It would be nice to use int32_t/uint32_t instead of int there,
since we in fact operate on ABI-defined type.
But I do not think it is much worry to expect an arch to appear which have
sizeof(int) != 4. It is already engraved in stone.
> - "struct msghdr32 *msg" depends on magic to translate first-level target
> 32-bit pointers to host N-bit pointers
> - uint32_t for `buf' and `from' is for pointers too. Now the magic for
> first-level pointers is not depended on
> - uint32_t for lengths is for size_t's. This is reasonable but would be
> clearer if spelled as size32_t (corresponding to l_size_t).
>
> >30 AUE_ACCEPT NOPROTO { int accept(int s, caddr_t name, \
> > int *anamelen); }
>
> As above for "int s". The use of caddr_t is more nonsense than usual.
> accept()'s second arg is not a caddr_t, but is "struct sockaddr *
> restrict". We depend first on type punning this to caddr_t, then to
> caddr_t actually being "char *", then on "char *" being a pointer,
> then on the usual magic for first-level pointers. kern/syscalls.master
> has the same caddr_t for the MCPT_NOA case of accept(), but not for
> the MSTD case. Perhaps userland accept() once actually took a caddr_t
> arg, but hopefully Linux is too new to have ever done that, and even
> compat cruft in FreeeBSD is too new to need that.
>
> >31 AUE_GETPEERNAME NOPROTO { int getpeername(int fdes, caddr_t asa, \
>
> Similarly. For getpeername(), the compat cruft in kern/syscalls.master
> is more clearly typed as MCOMPAT.
>
> >@@ -152,7 +152,7 @@
> >58 AUE_READLINK NOPROTO { ssize_t readlink(char *path, char *buf, \
> > size_t count); }
>
> All uses of basic C types and POSIX types are wronger than uses of
> fixed-width types. Here the size_t is not translated to uint32_t as
> above. This presumably works due to essentially the same magic as for
> first-level pointers, up to the copyout step: the 32-bit size_t gets
> extended to an N-bit one (where N is 32 or 64 on supported arches).
> Zero extension of it works right for size_t, and there is no further
> magic corresponding to the copy*() step.
>
> The magic is very convenient. It should probably be explicitly depended
> on for all first-level pointers. For size_t's it should be depended on
> in no cases or all cases.
>
> >59 AUE_EXECVE STD { int freebsd32_execve(char *fname, \
> >- u_int32_t *argv, u_int32_t *envv); }
> >+ uint32_t *argv, uint32_t *envv); }
>
> Usual hack for second-level pointers.
>
> >60 AUE_UMASK NOPROTO { int umask(int newmask); } umask \
> > umask_args int
>
> Usual bug for mode_t.
>
> >61 AUE_CHROOT NOPROTO { int chroot(char *path); }
> >@@ -325,10 +325,10 @@
> >172 AUE_NULL UNIMPL nosys
> >173 AUE_PREAD COMPAT6 { ssize_t freebsd32_pread(int fd, void *buf,
> >\
> > size_t nbyte, int pad, \
> >- u_int32_t offset1, u_int32_t offset2); }
> >+ uint32_t offset1, uint32_t offset2); }
>
> More confusing than usual. The size_t is translated by magic. Then there
> is bogus historical padding to be compatible with old mistakes in this
> area. Then there are 2 64-bit offsets (off_t's in the API) which are
> split because the translation code only understands 32-bit args.
>
> >[... stuff in which the largest obvious bugs are untranslated size_t's]
>
> >481 AUE_KILL NOPROTO { int thr_kill2(pid_t pid, long id, int
> >sig); }
>
> Untranslated pid_t depends on pid_t being no larger than int.
>
> Untranslated long completely breaks this.
>
> >482 AUE_SHMOPEN NOPROTO { int shm_open(const char *path, int flags, \
> >@@ -892,25 +892,25 @@
> >#ifdef PAD64_REQUIRED
> >485 AUE_NULL STD { int freebsd32_cpuset_setid(cpuwhich_t
> >which, \
> > int pad, \
> >- u_int32_t id1, u_int32_t id2, \
> >+ uint32_t id1, uint32_t id2, \
> > cpusetid_t setid); }
>
> Untranslated cpusetid_t. It works since it is int.
>
> >#else
> >485 AUE_NULL STD { int freebsd32_cpuset_setid(cpuwhich_t
> >which, \
> >- u_int32_t id1, u_int32_t id2, \
> >+ uint32_t id1, uint32_t id2, \
> > cpusetid_t setid); }
> >#endif
> >486 AUE_NULL STD { int freebsd32_cpuset_getid(cpulevel_t
> >level, \
> > cpuwhich_t which, \
> >- u_int32_t id1, u_int32_t id2, \
> >+ uint32_t id1, uint32_t id2, \
> > cpusetid_t *setid); }
>
> This and later have several more cpsetid_t's. Also cpuwhich_t's. id_t's
> are translated since they are 64 bits. This is painful. All types that
> are documented to be represented by id_t are only 32 bits, and id_t is
> wrong for representing 64-bit unsigned types since it is signed. The
> above has type puns to represent the signed 64-bit int in 2 uint32_t's.
> A pid of -1 would become pid_t -1, then id_t -1, then 0xffffffff,
> 0xffffffff. Signed off_t also requires type puns to represent in 2
> uint32_t's.
>
> >...
> >@@ -920,7 +920,7 @@
> >491 AUE_FCHOWNAT NOPROTO { int fchownat(int fd, char *path, uid_t
> >uid, \
> > gid_t gid, int flag); }
>
> Untranslated foo_t's.
>
> >@@ -959,9 +959,9 @@
> >512 AUE_SHMCTL NOSTD { int freebsd32_shmctl(int shmid, int cmd, \
> > struct shmid_ds32 *buf); }
> >513 AUE_LPATHCONF NOPROTO { int lpathconf(char *path, int name); }
> >-514 AUE_CAP_NEW NOPROTO { int cap_new(int fd, u_int64_t rights); }
> >+514 AUE_CAP_NEW NOPROTO { int cap_new(int fd, uint64_t rights); }
>
> Broken, assuming that all the other spittings into 2 uint32_t's are needed.
Yes, this is broken.
I added Robert and Jonathan to Cc:.
>
> >515 AUE_CAP_GETRIGHTS NOPROTO { int cap_getrights(int fd, \
> >- u_int64_t *rightsp); }
> >+ uint64_t *rightsp); }
>
> Now the uint64_t is indirect, so it pronbably works.
I looked at the sys_cap_getrights(), it seems to do the right thing.
>
> >Modified: head/sys/kern/syscalls.master
> >==============================================================================
> >--- head/sys/kern/syscalls.master Fri May 25 21:12:24 2012 (r236025)
> >+++ head/sys/kern/syscalls.master Fri May 25 21:50:48 2012 (r236026)
>
> I didn't think of uint*N_t in this at first either.
>
> >@@ -916,9 +916,9 @@
> >512 AUE_SHMCTL NOSTD { int shmctl(int shmid, int cmd, \
> > struct shmid_ds *buf); }
> >513 AUE_LPATHCONF STD { int lpathconf(char *path, int name); }
> >-514 AUE_CAP_NEW STD { int cap_new(int fd, u_int64_t rights); }
> >+514 AUE_CAP_NEW STD { int cap_new(int fd, uint64_t rights); }
> >515 AUE_CAP_GETRIGHTS STD { int cap_getrights(int fd, \
> >- u_int64_t *rightsp); }
> >+ uint64_t *rightsp); }
> >516 AUE_CAP_ENTER STD { int cap_enter(void); }
> >517 AUE_CAP_GETMODE STD { int cap_getmode(u_int *modep); }
> >518 AUE_PDFORK STD { int pdfork(int *fdp, int flags); }
>
> In fact, this file has only 3 uint*N_t's. This shows that these 3 are
> all API design bugs no matter how they are spelled. All the very old
> syscalls use basic C types. All the not so old ones use a STDC or
> POSIX typedefed type, with the typedef specific to the context. The
> 3 exceptions new ones that hard-code a fixed width are the above 2 CAP
> ones and sctp_peeloff(), which uses "uint32_t name".
>
> These bugs are easiest to fix in the Linux syscall.master's since the
> bugs are mostly already avoided using l_foo_t. In amd64/linux32/
> syscalls.master, I only noticed the following ones:
>
> % 1 AUE_EXIT NOPROTO { void sys_exit(int rval); } exit \
> % sys_exit_args void
>
> "int" for the return and args is not quite right, and it bogotifies the
> use of l_int for some args. It works though (assuming 32-bit host ints).
> Assumptions that the return type is 32-bit int and that the host int is
> 32-bits are implicit in all over, so we should assume them here for
> simplicity. (Hmm, td_retval is actually 2 register_t's. 64 bits each
> on amd64. amd64/linux32 has remarkably little code to convert these
> 2 64-bit ints into 4 32-bit ints.)
>
> % 2 AUE_FORK STD { int linux_fork(void); }
> % 3 AUE_NULL NOPROTO { int read(int fd, char *buf, \
> % u_int nbyte); }
>
> This u_int should strictly be l_size_t. l_size_t is used a lot elswhere
> in this file. In syscalls where the type is actually u_int, l_uint should
> be used. l_uint is used a lot elsewhere in this file. But not many
> syscsalls use u_int, so many of these l_uints are probably just different
> misspellings of l_size_t. So the density of logical type bugs is high
> even in this file.
>
> % 4 AUE_NULL NOPROTO { int write(int fd, char *buf, \
> % u_int nbyte); }
>
> u_int seems to be more common in older syscalls.
>
> % 5 AUE_OPEN_RWTC STD { int linux_open(char *path, l_int flags, \
> % l_int mode); }
>
> Finally, an example of perfectly correct use of l_int. It even correctly
> handles the subtlety that l_mode_t promotes to l_int since open() is
> variadic. (BTW, I don't like hard-coding this promotion and have
> wished for the __promoteof() operator for giving it for more than 20
> years. The promotion of a typedefed type even harder to determine than
> the correct PRI* mistake to use for printing a typedefed type.)
>
> This declaration is still far from correct altogether, since it depends
> on the usual hacks:
> - int return type is actual l_int
> - char *path is actually "const char *path" at the target level
> - magic to convert target "char *" to and from host "char *"
>
> % 11 AUE_EXECVE STD { int linux_execve(char *path, uint32_t
> *argp, \
> % uint32_t *envp); }
>
> See above.
>
> % 41 AUE_DUP NOPROTO { int dup(u_int fd); }
>
> Another untranslated type. dup() doesn't even take a u_int to begin with
> (it takes a normal int descriptor). Untranslated int args aren't actually
> very common.
>
> % 54 AUE_IOCTL STD { int linux_ioctl(l_uint fd, l_uint cmd, \
> % uintptr_t arg); }
>
> Now l_foo is used for the descriptor, but u_int is wrong -- Linux uses
> a normal int for the descriptor for ioctl() too.
>
> l_uint for cmd is wrong too. The type starts as u_long (same as in
> FreeBSD), at least in the 1997 Linux userland that I used to check
> this. This type mismatch is harmless because ints and longs are both
> 32 bits on Linux-i386.
>
> uintptr_t for arg is wronger. For ioctl(), this arg is the first variadic
> one and might not be present. I think declaring it in the above gives (via
> the Linux ABI) the garbage contents of a register for it when it isn't
> present. Native FreeBSD ioctl() has evem more magic. It misdeclares the
> arg as caddr_t instead, and gives stack garbage for it when it isn't
> present, and depends on the stack being large enough to avoid a memory
> fault when copying the stack garbage (the FreeBSD ABO gives at least a
> return address there).
>
> The type of the arg, when it is present, is specified to be caddr_t (:-()
> in FreeBSD. Linux (1997 userland) is better and says it is "char *". It
> should have been "void *". syscall.master's should probably declare it
> as "void *". Here it starts as a 32-bit pointer and the usual magic will
> convert it to a 64-bit non-pointer represented as a host uintptr_t by the
> above or by a host "void *" with "void *" in the above. copy*() will
> eventually turn it into "void *uaddr" either way. Then it will still
> not really be a kernel pointer, but is used as one in the implementation
> of copy*() on amd64 (this depends on a flat address space).
>
>
> % 55 AUE_FCNTL STD { int linux_fcntl(l_uint fd, l_uint cmd, \
> % uintptr_t arg); }
>
> Same errors for the types of fd and arg. Different error for the type of
> cmd. Unlike for ioctl(), it starts as neither long or unsigned, but just
> int (POSIX spec).
>
> % 56 AUE_NULL UNIMPL mpx
> % 57 AUE_SETPGRP NOPROTO { int setpgid(int pid, int pgid); }
>
> l_pid_t exists and should be used.
>
> % 60 AUE_UMASK NOPROTO { int umask(int newmask); }
>
> l_mode_t exists and should be used.
>
> % 63 AUE_DUP2 NOPROTO { int dup2(u_int from, u_int to); }
>
> Bogus fd types, as usual.
>
> % 65 AUE_GETPGRP NOPROTO { int getpgrp(void); }
>
> Should really be more careful with return types that are typedefed in the
> API (and are not just int statuses or fds). kern/syscalls.master is
> sloppy with this too.
>
> % 74 AUE_SYSCTL STD { int linux_sethostname(char *hostname, \
> % u_int len); }
>
> get/sethostname() is horribly inconsistent. In FreeBSD, gethostname() is
> documented as taking a size_t and sethostname() is documented as taking
> an int. kern/syscalls.master uses u_int for both. Apparently there is
> enough magic with arg packing for this to work even on supported
> 64-bit big-endian systems. Linux userland was better even in 1997 --
> it uses size_t for both. POSIX has specified gethostname() since at
> least 2001. It uses size_t of course. It says that these are 4.3BSD
> functions but it only specifies gethostname(), and it applies an
> OpenGroup resolution to change the type from socklen_t to size_t for
> gethostname(). This made it incompatible with BSD. Old BSD uses int
> for both, and FreeBSD apparently changed the documentation but not the
> code for gethostname() only, to be bug for compatible with POSIX.
>
> So the above should use l_size_t, and strictly, the code probably needs
> to be more careful than it is with unrepresentable sizes (native
> sethostname() wants a signed int and might not have the right checking
> for a 32-bit unsigned size_t).
>
> % 75 AUE_SETRLIMIT STD { int linux_setrlimit(l_uint resource, \
> % struct l_rlimit *rlim); }
> % 76 AUE_GETRLIMIT STD { int linux_old_getrlimit(l_uint resource, \
> % struct l_rlimit *rlim); }
>
> Resource numbers should be plain ints too. They are documented to be
> as such in old Linux userland man pages. But in old Linux (glibc)
> headers, they are obfuscated as __rlimit_resource_t, which an enum
> with gnu extensions, else plain int.
>
> % 78 AUE_NULL STD { int linux_gettimeofday( \
> % struct l_timeval *tp, \
> % struct timezone *tzp); }
> % 79 AUE_SETTIMEOFDAY STD { int linux_settimeofday( \
> % struct l_timeval *tp, \
> % struct timezone *tzp); }
>
> The timezone struct only has a couple of ints in it, so this abuse of
> the native timezone works, but is fragile. I forget if the timezone
> arg is deprecated to the point of ignoring it for native
> get/settimeofday(). If so, then this won't work for emulated
> get/settimeofday()
>
> % 85 AUE_READLINK STD { int linux_readlink(char *name, char *buf, \
> % l_int count); }
>
> Native readlink was churned by POSIX from an int count to a size_t count.
> Linux readlink has takes a size_t count even in 1997 and probably still
> does.
>
> % 88 AUE_REBOOT STD { int linux_reboot(l_int magic1, \
> % l_int magic2, l_uint cmd, void *arg); }
>
> In old Linux userland it is documented to take only 3 int args, with the
> 3rd one named `flag'.
>
> % ; 89: old_readdir
> % 89 AUE_GETDIRENTRIES STD { int linux_readdir(l_uint fd, \
> % struct l_dirent *dent, l_uint count); }
>
> Matches old Linux userland man page right down to the u_int for fd.
>
>
> % 91 AUE_MUNMAP NOPROTO { int munmap(caddr_t addr, int len); }
>
> Takes a void * and a size_t in modern munmap() and Linux had that in 1997.
> FreeBSD-3 had this too. The above matches FreeBSD-1.
>
> % 92 AUE_TRUNCATE STD { int linux_truncate(char *path, \
> % l_ulong length); }
>
> Should use l_off_t, not l_ulong (except possibly if their is an unsigned
> hack to support file sizes of 2G-4G).
>
> % 93 AUE_FTRUNCATE STD { int linux_ftruncate(int fd, long length); }
>
> Using long is very broken. l_off_t is correct here too.
>
> % 94 AUE_FCHMOD NOPROTO { int fchmod(int fd, int mode); }
>
> mode_t is used for all other *chmod*()'s in this file
>
> % 95 AUE_FCHOWN NOPROTO { int fchown(int fd, int uid, int gid); }
>
> l_uid16_t and l_gid16_t are used for all other old *chown*() in this file.
> (Newer ones are spelled *lchown*() and take l_uid_t and l_gid_t's.)
>
> % 96 AUE_GETPRIORITY STD { int linux_getpriority(int which, int who);
> }
> % 97 AUE_SETPRIORITY NOPROTO { int setpriority(int which, int who, \
> % int prio); }
>
> `who' should have type pid_t, but is still int in FreeBSD native. In
> old Linux, it is int in the man page but is obfuscated as id_t in the
> glibc header. Even `which' is obfuscated as __priority_which_t in the
> glibc header. (I consider even using size_t for read() to be a
> regression. Typedefed types are hard to use correctly, as shown by
> this mail which points out that hundreds of incorrect uses, where the
> bugs are only harmless because the use of typedefed types makes no
> significant difference.)
>
> Got bored here, and looked at the rest less carefully. The newer
> syscalls seem to be handled more carefully, probably because plenty
> of Linux types existed when they were written. There just seemed to
> be too many l_[u]longs in them.
>
> % 168 AUE_POLL NOPROTO { int poll(struct pollfd *fds, \
> % unsigned int nfds, int timeout); }
>
> Hrmph, this is the only one that misspells "unsigned" more verbosely as
> "unsigned int" instead of as the usual KNF abbreviation "u_int".
>
> Of course, poll() doesn't even take a u_int type. Modern poll takes
> an nfds_t type. Linux had that in 1997, and it was u_long, not u_int.
>
> Use of the native struct pollfd is dangerous. It works because it
> consists of an int followed by 2 shorts, and this matches the Linux
> type and neither has neither unnamed padding on at least amd64 and i386.
>
> % 180 AUE_PREAD STD { int linux_pread(l_uint fd, char *buf, \
> % l_size_t nbyte, l_loff_t offset); }
> % [... several more bogus l_uints for fds]
>
> % 183 AUE_GETCWD STD { int linux_getcwd(char *buf, \
> % l_ulong bufsize); }
>
> A different misspelling of l_size_t.
>
> % 191 AUE_GETRLIMIT STD { int linux_getrlimit(l_uint resource, \
> % struct l_rlimit *rlim); }
>
> Bogus resource number times for new rlimit syscalls too.
>
> % 193 AUE_TRUNCATE STD { int linux_truncate64(char *path, \
> % l_loff_t length); }
> % 194 AUE_FTRUNCATE STD { int linux_ftruncate64(l_uint fd, \
> % l_loff_t length); }
>
> How can this work? I was going to say that the splitting up of the 64-bit
> args in compat/freebsd32/syscalls.master was unnecessary because it is
> apparently not needed here. Then I misread this as doing the splitting.
> But it doesn't. Similarly later in this file.
>
> % 218 AUE_MINCORE STD { int linux_mincore(l_ulong start, \
> % l_size_t len, u_char *vec); }
> % 219 AUE_MADVISE NOPROTO { int madvise(void *addr, size_t len, \
> % int behav); }
>
> SIlly to have l_size_t in one and raw size_t in the next.
>
> I just noticed that NOPROTO calls never use l_foo. They go direct to
> the native calls, and spelling things without an l_ may be necessary
> for hiding the type mismatches from this. But this is especially
> fragile, and maybe the l_'s here would be harmless since they have no
> effect, since type checking for the direct calls is destroyed better
> using (sy_call_t *) casts.
>
> % 220 AUE_GETDIRENTRIES STD { int linux_getdents64(l_uint fd, \
> % void *dirent, l_uint count); }
> % 221 AUE_FCNTL STD { int linux_fcntl64(l_uint fd, l_uint cmd, \
> % uintptr_t arg); }
>
> Type errors for 32-bit syscalls are often duplicated for the corresponding
> 64-bit ones. My old userland doesn't have these new syscalls to check,
> but I doubt that Linux changed the API for the non-64 bit parts, and
> u_int fds are bogus as usual.
>
> % 240 AUE_NULL STD { int linux_sys_futex(void *uaddr, int op,
> uint32_t val, \
> % struct l_timespec *timeout, uint32_t
> *uaddr2, uint32_t val3); }
> % 241 AUE_NULL STD { int linux_sched_setaffinity(l_pid_t pid,
> l_uint len, \
> % l_ulong *user_mask_ptr); }
> % 242 AUE_NULL STD { int linux_sched_getaffinity(l_pid_t pid,
> l_uint len, \
> % l_ulong *user_mask_ptr); }
> % 243 AUE_NULL STD { int linux_set_thread_area(struct
> l_user_desc *desc); }
>
> Lots of style bugs which are not present in old code:
> - lines are not wrapped at 80 columns
> - wrapped lines are misindented.
>
>
> % 250 AUE_NULL STD { int linux_fadvise64(int fd, l_loff_t
> offset, \
> % l_size_t len, int advice); }
>
> More style bugs.
>
> % 264 AUE_CLOCK_SETTIME STD { int linux_clock_settime(clockid_t
> which, struct l_timespec *tp); }
> % 265 AUE_NULL STD { int linux_clock_gettime(clockid_t which,
> struct l_timespec *tp); }
> % 266 AUE_NULL STD { int linux_clock_getres(clockid_t which,
> struct l_timespec *tp); }
> % 267 AUE_NULL STD { int linux_clock_nanosleep(clockid_t which,
> int flags, \
> % struct l_timespec *rqtp, struct
> l_timespec *rmtp); }
> % 268 AUE_STATFS STD { int linux_statfs64(char *path, size_t
> bufsize, struct l_statfs64_buf *buf); }
>
> Another raw size_t.
>
> Another long line.
>
> % 269 AUE_FSTATFS STD { int linux_fstatfs64(void); }
> % 270 AUE_NULL STD { int linux_tgkill(int tgid, int pid, int
> sig); }
>
> Raw ints for pids.
>
> % 271 AUE_UTIMES STD { int linux_utimes(char *fname, \
> % struct l_timeval *tptr); }
> % 272 AUE_NULL STD { int linux_fadvise64_64(int fd, \
> % l_loff_t offset, l_loff_t len, \
> % int advice); }
>
> Just about all syscalls added since #239 are misformatted :-(.
>
> % 295 AUE_OPEN_RWTC STD { int linux_openat(l_int dfd, const char
> *filename, \
> % l_int flags, l_int mode); }
> % 296 AUE_MKDIRAT STD { int linux_mkdirat(l_int dfd, const char
> *pathname, \
> % l_int mode); }
> % 297 AUE_MKNODAT STD { int linux_mknodat(l_int dfd, const char
> *filename, \
> % l_int mode, l_uint dev); }
> % 298 AUE_FCHOWNAT STD { int linux_fchownat(l_int dfd, const char
> *filename, \
> % l_uid16_t uid, l_gid16_t gid, l_int
> flag); }
> % 299 AUE_FUTIMESAT STD { int linux_futimesat(l_int dfd, char
> *filename, \
> % struct l_timeval *utimes); }
> % 300 AUE_FSTATAT STD { int linux_fstatat64(l_int dfd, char
> *pathname, \
> % struct l_stat64 *statbuf, l_int
> flag); }
> % 301 AUE_UNLINKAT STD { int linux_unlinkat(l_int dfd, const char
> *pathname, \
> % l_int flag); }
> % 302 AUE_RENAMEAT STD { int linux_renameat(l_int olddfd, const
> char *oldname, \
> % l_int newdfd, const char *newname); }
> % 303 AUE_LINKAT STD { int linux_linkat(l_int olddfd, const char
> *oldname, \
> % l_int newdfd, const char *newname,
> l_int flag); }
> % 304 AUE_SYMLINKAT STD { int linux_symlinkat(const char *oldname,
> l_int newdfd, \
> % const char *newname); }
> % 305 AUE_READLINKAT STD { int linux_readlinkat(l_int dfd, const char
> *path, \
> % char *buf, l_int bufsiz); }
> % 306 AUE_FCHMODAT STD { int linux_fchmodat(l_int dfd, const char
> *filename, \
> % l_mode_t mode); }
> % 307 AUE_FACCESSAT STD { int linux_faccessat(l_int dfd, const char
> *filename, l_int amode, int flag); }
>
> Mounds more misformatting.
>
> [... A small pile of misformatting]
>
> Bruce
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20120526/59283986/attachment.pgp
More information about the svn-src-all
mailing list