From 2ea0a58e01c1ed6db1da9dd0314ee053f5a32026 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 25 Jul 2014 17:24:00 -0700 Subject: [PATCH] Fix linkage of grantpt(3). Also clean up the implementation of all the pty functions, add tests, and fix the stub implementations of ttyname(3) and ttyname_r(3). Bug: https://code.google.com/p/android/issues/detail?id=58888 (cherry picked from commit 4916706cfe590eb06c9b5bd4bd402ce056034d51) Change-Id: I5cb7a1c17b156456e4c4818e65f256eb8d045424 --- libc/Android.mk | 5 +- libc/bionic/getpt.c | 34 --------- libc/bionic/ptsname.c | 44 ----------- libc/bionic/{ptsname_r.c => pty.cpp} | 105 ++++++++++++++++++++------- libc/bionic/stubs.cpp | 10 --- libc/bionic/unlockpt.c | 36 --------- libc/include/stdlib.h | 18 ++--- libc/include/unistd.h | 2 +- tests/stdlib_test.cpp | 99 +++++++++++++++++++++++++ 9 files changed, 184 insertions(+), 169 deletions(-) delete mode 100644 libc/bionic/getpt.c delete mode 100644 libc/bionic/ptsname.c rename libc/bionic/{ptsname_r.c => pty.cpp} (53%) delete mode 100644 libc/bionic/unlockpt.c diff --git a/libc/Android.mk b/libc/Android.mk index 0487a8fc0..15a68b921 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -45,7 +45,6 @@ libc_common_src_files := \ bionic/fts.c \ bionic/gethostname.c \ bionic/getpriority.c \ - bionic/getpt.c \ bionic/if_indextoname.c \ bionic/if_nametoindex.c \ bionic/initgroups.c \ @@ -53,8 +52,6 @@ libc_common_src_files := \ bionic/isatty.c \ bionic/memmem.c \ bionic/pathconf.c \ - bionic/ptsname.c \ - bionic/ptsname_r.c \ bionic/pututline.c \ bionic/sched_cpualloc.c \ bionic/sched_cpucount.c \ @@ -63,7 +60,6 @@ libc_common_src_files := \ bionic/siginterrupt.c \ bionic/sigsetmask.c \ bionic/system_properties_compat.c \ - bionic/unlockpt.c \ stdio/snprintf.c\ stdio/sprintf.c \ @@ -175,6 +171,7 @@ libc_bionic_src_files := \ bionic/pthread_setschedparam.cpp \ bionic/pthread_sigmask.cpp \ bionic/ptrace.cpp \ + bionic/pty.cpp \ bionic/raise.cpp \ bionic/rand.cpp \ bionic/readlink.cpp \ diff --git a/libc/bionic/getpt.c b/libc/bionic/getpt.c deleted file mode 100644 index 8bb5c1138..000000000 --- a/libc/bionic/getpt.c +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2008 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ -#include -#include - -int getpt(void) -{ - return open("/dev/ptmx", O_RDWR|O_NOCTTY); -} diff --git a/libc/bionic/ptsname.c b/libc/bionic/ptsname.c deleted file mode 100644 index 24d5d3099..000000000 --- a/libc/bionic/ptsname.c +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (C) 2008 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ -#include -#include -#include -#include - -/* not thread-safe */ -char* ptsname( int fd ) -{ - unsigned int pty_num; - static char buff[64]; - - if ( ioctl( fd, TIOCGPTN, &pty_num ) != 0 ) - return NULL; - - snprintf( buff, sizeof(buff), "/dev/pts/%u", pty_num ); - return buff; -} diff --git a/libc/bionic/ptsname_r.c b/libc/bionic/pty.cpp similarity index 53% rename from libc/bionic/ptsname_r.c rename to libc/bionic/pty.cpp index 2fa4c3d34..995e00608 100644 --- a/libc/bionic/ptsname_r.c +++ b/libc/bionic/pty.cpp @@ -25,34 +25,83 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ -#include -#include -#include -#include + #include -#include +#include +#include +#include +#include +#include +#include -int ptsname_r( int fd, char* buf, size_t buflen) -{ - unsigned int pty_num; - char buff[64]; - int len; - - if (buf == NULL) { - errno = EINVAL; - return -1; - } - - if ( ioctl( fd, TIOCGPTN, &pty_num ) != 0 ) { - errno = ENOTTY; - return -1; - } - - len = snprintf( buff, sizeof(buff), "/dev/pts/%u", pty_num ); - if (len+1 > (int)buflen) { - errno = ERANGE; - return -1; - } - memcpy( buf, buff, len+1 ); - return 0; +int getpt(void) { + return posix_openpt(O_RDWR|O_NOCTTY); +} + +int grantpt(int) { + return 0; +} + +int posix_openpt(int flags) { + return open("/dev/ptmx", flags); +} + +char* ptsname(int fd) { + static char buf[64]; + return ptsname_r(fd, buf, sizeof(buf)) == 0 ? buf : NULL; +} + +int ptsname_r(int fd, char* buf, size_t len) { + if (buf == NULL) { + errno = EINVAL; + return errno; + } + + unsigned int pty_num; + if (ioctl(fd, TIOCGPTN, &pty_num) != 0) { + errno = ENOTTY; + return errno; + } + + if (snprintf(buf, len, "/dev/pts/%u", pty_num) >= static_cast(len)) { + errno = ERANGE; + return errno; + } + + return 0; +} + +char* ttyname(int fd) { + static char buf[64]; + return ttyname_r(fd, buf, sizeof(buf)) == 0 ? buf : NULL; +} + +int ttyname_r(int fd, char* buf, size_t len) { + if (buf == NULL) { + errno = EINVAL; + return errno; + } + + if (!isatty(fd)) { + return errno; + } + + char path[64]; + snprintf(path, sizeof(path), "/proc/self/fd/%d", fd); + + ssize_t count = readlink(path, buf, len); + if (count == -1) { + return errno; + } + if (static_cast(count) == len) { + errno = ERANGE; + return errno; + } + buf[count] = '\0'; + return 0; +} + +int unlockpt(int fd) { + int unlock = 0; + return ioctl(fd, TIOCSPTLCK, &unlock); } diff --git a/libc/bionic/stubs.cpp b/libc/bionic/stubs.cpp index 0937e9c06..b1e38be8f 100644 --- a/libc/bionic/stubs.cpp +++ b/libc/bionic/stubs.cpp @@ -444,16 +444,6 @@ void endpwent() { UNIMPLEMENTED; } -char* ttyname(int /*fd*/) { // NOLINT: implementing bad function. - UNIMPLEMENTED; - return NULL; -} - -int ttyname_r(int /*fd*/, char* /*buf*/, size_t /*buflen*/) { - UNIMPLEMENTED; - return -ERANGE; -} - char* getusershell() { UNIMPLEMENTED; return NULL; diff --git a/libc/bionic/unlockpt.c b/libc/bionic/unlockpt.c deleted file mode 100644 index 998b7a312..000000000 --- a/libc/bionic/unlockpt.c +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (C) 2008 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ -#include -#include - -int unlockpt( int fd ) -{ - int unlock = 0; - - return ioctl( fd, TIOCSPTLCK, &unlock ); -} diff --git a/libc/include/stdlib.h b/libc/include/stdlib.h index 195765aea..857d6313f 100644 --- a/libc/include/stdlib.h +++ b/libc/include/stdlib.h @@ -120,18 +120,12 @@ long random(void); char* setstate(char*); void srandom(unsigned int); -/* Basic PTY functions. These only work if devpts is mounted! */ - -extern int unlockpt(int); -extern char* ptsname(int); -extern int ptsname_r(int, char*, size_t); -extern int getpt(void); - -static __inline__ int grantpt(int __fd __attribute((unused))) -{ - (void)__fd; - return 0; /* devpts does this all for us! */ -} +int getpt(void); +int grantpt(int); +int posix_openpt(int); +char* ptsname(int) __warnattr("ptsname is not thread-safe; use ptsname_r instead"); +int ptsname_r(int, char*, size_t); +int unlockpt(int); typedef struct { int quot; diff --git a/libc/include/unistd.h b/libc/include/unistd.h index 12e625743..82c53e813 100644 --- a/libc/include/unistd.h +++ b/libc/include/unistd.h @@ -170,7 +170,7 @@ extern char *optarg; extern int optind, opterr, optopt; extern int isatty(int); -extern char* ttyname(int); +extern char* ttyname(int) __warnattr("ttyname is not thread-safe; use ttyname_r instead"); extern int ttyname_r(int, char*, size_t); extern int acct(const char* filepath); diff --git a/tests/stdlib_test.cpp b/tests/stdlib_test.cpp index 6d29421d0..553f018ed 100644 --- a/tests/stdlib_test.cpp +++ b/tests/stdlib_test.cpp @@ -262,3 +262,102 @@ TEST(unistd, _Exit) { ASSERT_TRUE(WIFEXITED(status)); ASSERT_EQ(99, WEXITSTATUS(status)); } + +TEST(stdlib, pty_smoke) { + // getpt returns a pty with O_RDWR|O_NOCTTY. + int fd = getpt(); + ASSERT_NE(-1, fd); + + // grantpt is a no-op. + ASSERT_EQ(0, grantpt(fd)); + + // ptsname_r should start "/dev/pts/". + char name_r[128]; + ASSERT_EQ(0, ptsname_r(fd, name_r, sizeof(name_r))); + name_r[9] = 0; + ASSERT_STREQ("/dev/pts/", name_r); + + close(fd); +} + +TEST(stdlib, posix_openpt) { + int fd = posix_openpt(O_RDWR|O_NOCTTY|O_CLOEXEC); + ASSERT_NE(-1, fd); + close(fd); +} + +TEST(stdlib, ptsname_r_ENOTTY) { + errno = 0; + char buf[128]; + ASSERT_EQ(ENOTTY, ptsname_r(STDOUT_FILENO, buf, sizeof(buf))); + ASSERT_EQ(ENOTTY, errno); +} + +TEST(stdlib, ptsname_r_EINVAL) { + int fd = getpt(); + ASSERT_NE(-1, fd); + errno = 0; + char* buf = NULL; + ASSERT_EQ(EINVAL, ptsname_r(fd, buf, 128)); + ASSERT_EQ(EINVAL, errno); + close(fd); +} + +TEST(stdlib, ptsname_r_ERANGE) { + int fd = getpt(); + ASSERT_NE(-1, fd); + errno = 0; + char buf[1]; + ASSERT_EQ(ERANGE, ptsname_r(fd, buf, sizeof(buf))); + ASSERT_EQ(ERANGE, errno); + close(fd); +} + +TEST(stdlib, ttyname_r) { + int fd = getpt(); + ASSERT_NE(-1, fd); + + // ttyname_r returns "/dev/ptmx" for a pty. + char name_r[128]; + ASSERT_EQ(0, ttyname_r(fd, name_r, sizeof(name_r))); + ASSERT_STREQ("/dev/ptmx", name_r); + + close(fd); +} + +TEST(stdlib, ttyname_r_ENOTTY) { + int fd = open("/dev/null", O_WRONLY); + errno = 0; + char buf[128]; + ASSERT_EQ(ENOTTY, ttyname_r(fd, buf, sizeof(buf))); + ASSERT_EQ(ENOTTY, errno); + close(fd); +} + +TEST(stdlib, ttyname_r_EINVAL) { + int fd = getpt(); + ASSERT_NE(-1, fd); + errno = 0; + char* buf = NULL; + ASSERT_EQ(EINVAL, ttyname_r(fd, buf, 128)); + ASSERT_EQ(EINVAL, errno); + close(fd); +} + +TEST(stdlib, ttyname_r_ERANGE) { + int fd = getpt(); + ASSERT_NE(-1, fd); + errno = 0; + char buf[1]; + ASSERT_EQ(ERANGE, ttyname_r(fd, buf, sizeof(buf))); + ASSERT_EQ(ERANGE, errno); + close(fd); +} + +TEST(stdlib, unlockpt_ENOTTY) { + int fd = open("/dev/null", O_WRONLY); + errno = 0; + ASSERT_EQ(-1, unlockpt(fd)); + ASSERT_EQ(ENOTTY, errno); + close(fd); +}