Date: Fri, 15 Jun 2018 14:34:44 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Brooks Davis <brooks@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r335177 - in head/sys: compat/freebsd32 compat/linux i386/ibcs2 kern vm Message-ID: <20180615135333.C1111@besplex.bde.org> In-Reply-To: <201806142127.w5ELRPoK022899@repo.freebsd.org> References: <201806142127.w5ELRPoK022899@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 14 Jun 2018, Brooks Davis wrote:
> ...
> Log:
> Name the implementation of brk and sbrk sys_break().
This API was actually named brk(2).
Now brk(2) doesn't actually give this API. brk(2) doesn't exist in libc.
Only brk(3) exists. It calls the actual brk(2) and adds complications.
brk(3) doesn't exist according to its (nonexistent) man page.
sbrk(2) hasn't existed in libc for much longer. Only sbrk(3) exists. It
calls brk(3nonexistent) and adds complications. sbrk(3) doesn't exist
according to its (nonexistent) man page.
> ...
> Modified: head/sys/compat/freebsd32/syscalls.master
> ==============================================================================
> --- head/sys/compat/freebsd32/syscalls.master Thu Jun 14 21:22:14 2018 (r335176)
> +++ head/sys/compat/freebsd32/syscalls.master Thu Jun 14 21:27:25 2018 (r335177)
> @@ -87,8 +87,7 @@
> int mode, int dev); }
> 15 AUE_CHMOD NOPROTO { int chmod(char *path, int mode); }
> 16 AUE_CHOWN NOPROTO { int chown(char *path, int uid, int gid); }
> -17 AUE_NULL NOPROTO { caddr_t obreak(char *nsize); } break \
> - obreak_args int
> +17 AUE_NULL NOPROTO { caddr_t break(char *nsize); }
> 18 AUE_GETFSSTAT COMPAT4 { int freebsd32_getfsstat( \
> struct statfs32 *buf, long bufsize, \
> int mode); }
>
brk(2) under any name returns int, not caddr_t. Even its man page gets this
correct. This was broken for all cases except one.
The type checking of declarations generated by these pseudo-prototypes is
too weak to detect the error. First the good name of these APIs is
broken by adding a sys_ prefix. I think the return type is ignored.
Syscall implementation functions all return int (just an error code).
They return actual values in td_retval. Indirections break type checking
for this especially well. Old versions that passed around a pointer to
td_retval could have done type checking much like is done for args structs.
The type checking is also weak for args structs, and this is useful for
avoiding const poisoning. Type qualifiers get lost somewhere even when
they are in the pseudo-prototypes. Lots of other errors like [signed]
int instead of [unsigned] uid_t visible for chown() in the above go
undetected. Even the size mismatch for 16-bit mode_t vs 32-bit int
visible in the above for chmod() goes undetected.
> Modified: head/sys/i386/ibcs2/syscalls.master
> ==============================================================================
> --- head/sys/i386/ibcs2/syscalls.master Thu Jun 14 21:22:14 2018 (r335176)
> +++ head/sys/i386/ibcs2/syscalls.master Thu Jun 14 21:27:25 2018 (r335177)
> @@ -57,7 +57,7 @@
> 15 AUE_CHMOD STD { int ibcs2_chmod(char *path, int mode); }
> 16 AUE_CHOWN STD { int ibcs2_chown(char *path, int uid, \
> int gid); }
> -17 AUE_NULL NOPROTO { int obreak(caddr_t nsize); }
> +17 AUE_NULL NOPROTO { caddr_t break(caddr_t nsize); }
> 18 AUE_STAT STD { int ibcs2_stat(char* path, \
> struct ibcs2_stat *st); }
> 19 AUE_LSEEK STD { long ibcs2_lseek(int fd, long offset, \
This was the one instance with the correct return type.
All of the arg types for break() are broken too. This one is different,
but the difference happens to be just a misspelling of the same wrong
type.
brk(2) is documented to have the newfangled arg type void *. The
implementation of [s]brk(3) uses the correct type, but the
syscalls.master files haven't caught up yet. Most use char *, but
this one uses caddr_t (which happens to be char *). Perhaps caddr_t
is actually correct for ibcs2. But then it would be the ibcs2 caddr_t
that is correct.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180615135333.C1111>