Date: Sat, 3 Jan 2015 18:39:48 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dmitry Chagin <dchagin@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r276564 - head/sys/compat/linux Message-ID: <20150103175627.K979@besplex.bde.org> In-Reply-To: <201501021929.t02JTXd7093292@svn.freebsd.org> References: <201501021929.t02JTXd7093292@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 2 Jan 2015, Dmitry Chagin wrote:
> Log:
> Cast *path to silence clang -Wpointer-sign warning.
Why not fix the bug instead of breaking the warning? (You usually do
this better.)
> Modified: head/sys/compat/linux/linux_getcwd.c
> ==============================================================================
> --- head/sys/compat/linux/linux_getcwd.c Fri Jan 2 19:06:27 2015 (r276563)
> +++ head/sys/compat/linux/linux_getcwd.c Fri Jan 2 19:29:32 2015 (r276564)
> @@ -430,7 +430,7 @@ linux_getcwd(struct thread *td, struct l
>
> path = (char *)malloc(len, M_TEMP, M_WAITOK);
>
> - error = kern___getcwd(td, path, UIO_SYSSPACE, len);
> + error = kern___getcwd(td, (u_char *)path, UIO_SYSSPACE, len);
The bug is that __getcwd(2undoc) has a bogus API that is mismatched with
all callers and also with its implementation.
libc/gen/getcwd.c misdeclares __getcwd() as taking a plain char * path, but
this type mismatch is not detected since libc/gen/getcwd.c doesn't include
the header where the syscall is declared (sys/syscallsubr.h). This type
pun prevents clang detecting the same problem as above (pathnames are
always char * except when bogusly passed to *__getcwd()).
__kern_getcwd() internally doesn't actually use or support u_char * paths,
except to copy them to a normal char * path. The copying is done using
bcopy(), so its behaviour is not undefined like it is with the type pun,
but it is still logically wrong and would be physically wrong if signed
chars were exotic. The behaviour is only clearly correct if the pointer:
1) starts as plain char *
2) actually points to valid plain chars
3) is converted without type puns before use, either back to the original
plain char * pointer for direct use, or to void * for use in bcopy()
and then plain char * is used to access the copy.
The above bogus cast gives this. This depends only __kern_getcwd() not
actually supporting u_char * but doing step (3).
I think the bogus cast from 'signed foo_t *' to 'unsigned foo_t *' gives
undefined behaviour in general, but for foo_t = char it only gives
implementation-defined behaviour (different interpretations of the sign
bit and +-0), at least in POSIX where chars are 8 bits so there is no
space for extra bits. The only case of even theoretical interest is
+-0 on 1's complement systems with plain char = signed char . Then +-0
cannot be distinguished as chars. FreeBSD just doesn't support such
systems. At least to the extent of handling the file name "\xff" on
a removable disk generated on a 2's complement system.
The cast of malloc() is also bogus.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150103175627.K979>