Cleanup for #inclusivefixit.
This was initially just to try on "pty" and "tty" for size, while
discussing with other projects to try to align on vocabulary, but -- in
the implemention and tests at least -- but these work out so well that
I'm tempted to go with them anyway if we can't come to a concensus. We
can always come back and change them again later.
What I really like is that you pass the pty to functions like ptsname()
and grantpt() with a 'p' in the name, and the tty to functions like
ttyname() and tcsetattr() with just 't's.
The use of "parent" and "child" in forkpty() seems helpful too.
Also fix the documentation of forkpty(), which wasn't quite right.
Test: treehugger
Change-Id: Ic010c4b669f6528591c653e3701f4e41e0d0df9e
diff --git a/libc/bionic/pty.cpp b/libc/bionic/pty.cpp
index 71e14d9..c0e2721 100644
--- a/libc/bionic/pty.cpp
+++ b/libc/bionic/pty.cpp
@@ -112,14 +112,14 @@
return ioctl(fd, TIOCSPTLCK, &unlock);
}
-int openpty(int* master, int* slave, char* name, const termios* t, const winsize* ws) {
- *master = getpt();
- if (*master == -1) {
+int openpty(int* pty, int* tty, char* name, const termios* t, const winsize* ws) {
+ *pty = getpt();
+ if (*pty == -1) {
return -1;
}
- if (grantpt(*master) == -1 || unlockpt(*master) == -1) {
- close(*master);
+ if (grantpt(*pty) == -1 || unlockpt(*pty) == -1) {
+ close(*pty);
return -1;
}
@@ -127,54 +127,54 @@
if (name == nullptr) {
name = buf;
}
- if (ptsname_r(*master, name, sizeof(buf)) != 0) {
- close(*master);
+ if (ptsname_r(*pty, name, sizeof(buf)) != 0) {
+ close(*pty);
return -1;
}
- *slave = open(name, O_RDWR|O_NOCTTY);
- if (*slave == -1) {
- close(*master);
+ *tty = open(name, O_RDWR | O_NOCTTY);
+ if (*tty == -1) {
+ close(*pty);
return -1;
}
if (t != nullptr) {
- tcsetattr(*slave, TCSAFLUSH, t);
+ tcsetattr(*tty, TCSAFLUSH, t);
}
if (ws != nullptr) {
- ioctl(*slave, TIOCSWINSZ, ws);
+ ioctl(*tty, TIOCSWINSZ, ws);
}
return 0;
}
-int forkpty(int* amaster, char* name, const termios* t, const winsize* ws) {
- int master;
- int slave;
- if (openpty(&master, &slave, name, t, ws) == -1) {
+int forkpty(int* parent_pty, char* child_tty_name, const termios* t, const winsize* ws) {
+ int pty;
+ int tty;
+ if (openpty(&pty, &tty, child_tty_name, t, ws) == -1) {
return -1;
}
pid_t pid = fork();
if (pid == -1) {
- close(master);
- close(slave);
+ close(pty);
+ close(tty);
return -1;
}
if (pid == 0) {
// Child.
- *amaster = -1;
- close(master);
- if (login_tty(slave) == -1) {
+ *parent_pty = -1;
+ close(pty);
+ if (login_tty(tty) == -1) {
_exit(1);
}
return 0;
}
// Parent.
- *amaster = master;
- close(slave);
+ *parent_pty = pty;
+ close(tty);
return pid;
}
diff --git a/libc/include/pty.h b/libc/include/pty.h
index 90d6686..be447d6 100644
--- a/libc/include/pty.h
+++ b/libc/include/pty.h
@@ -49,16 +49,17 @@
*
* Available since API level 23.
*/
-int openpty(int* _Nonnull __master_fd, int* _Nonnull __slave_fd, char* _Nullable __slave_name, const struct termios* _Nullable __termios_ptr, const struct winsize* _Nullable __winsize_ptr) __INTRODUCED_IN(23);
+int openpty(int* _Nonnull __pty_fd, int* _Nonnull __tty_fd, char* _Nullable __tty_name, const struct termios* _Nullable __termios_ptr, const struct winsize* _Nullable __winsize_ptr) __INTRODUCED_IN(23);
/**
* [forkpty(3)](http://man7.org/linux/man-pages/man3/forkpty.3.html) creates
* a new process connected to a pseudoterminal from openpty().
*
- * Returns 0 on success and returns -1 and sets `errno` on failure.
+ * Returns 0 in the child/the pid of the child in the parent on success,
+ * and returns -1 and sets `errno` on failure.
*
* Available since API level 23.
*/
-int forkpty(int* _Nonnull __master_fd, char* _Nullable __slave_name, const struct termios* _Nullable __termios_ptr, const struct winsize* _Nullable __winsize_ptr) __INTRODUCED_IN(23);
+int forkpty(int* _Nonnull __parent_pty_fd, char* _Nullable __child_tty_name, const struct termios* _Nullable __termios_ptr, const struct winsize* _Nullable __winsize_ptr) __INTRODUCED_IN(23);
__END_DECLS
diff --git a/tests/pty_test.cpp b/tests/pty_test.cpp
index 29f86f1..d5d8994 100644
--- a/tests/pty_test.cpp
+++ b/tests/pty_test.cpp
@@ -28,34 +28,34 @@
#include "utils.h"
TEST(pty, openpty) {
- int master, slave;
+ int pty, tty;
char name[32];
struct winsize w = { 123, 456, 9999, 999 };
- ASSERT_EQ(0, openpty(&master, &slave, name, nullptr, &w));
- ASSERT_NE(-1, master);
- ASSERT_NE(-1, slave);
- ASSERT_NE(master, slave);
+ ASSERT_EQ(0, openpty(&pty, &tty, name, nullptr, &w));
+ ASSERT_NE(-1, pty);
+ ASSERT_NE(-1, tty);
+ ASSERT_NE(pty, tty);
char tty_name[32];
- ASSERT_EQ(0, ttyname_r(slave, tty_name, sizeof(tty_name)));
+ ASSERT_EQ(0, ttyname_r(tty, tty_name, sizeof(tty_name)));
ASSERT_STREQ(tty_name, name);
struct winsize w_actual;
- ASSERT_EQ(0, ioctl(slave, TIOCGWINSZ, &w_actual));
+ ASSERT_EQ(0, ioctl(tty, TIOCGWINSZ, &w_actual));
ASSERT_EQ(w_actual.ws_row, w.ws_row);
ASSERT_EQ(w_actual.ws_col, w.ws_col);
ASSERT_EQ(w_actual.ws_xpixel, w.ws_xpixel);
ASSERT_EQ(w_actual.ws_ypixel, w.ws_ypixel);
- close(master);
- close(slave);
+ close(pty);
+ close(tty);
}
TEST(pty, forkpty) {
pid_t sid = getsid(0);
- int master;
- pid_t pid = forkpty(&master, nullptr, nullptr, nullptr);
+ int pty;
+ pid_t pid = forkpty(&pty, nullptr, nullptr, nullptr);
ASSERT_NE(-1, pid);
if (pid == 0) {
@@ -68,12 +68,12 @@
AssertChildExited(pid, 0);
- close(master);
+ close(pty);
}
struct PtyReader_28979140_Arg {
int main_cpu_id;
- int slave_fd;
+ int fd;
uint32_t data_count;
bool finished;
std::atomic<bool> matched;
@@ -90,7 +90,7 @@
while (counter <= arg->data_count) {
char buf[4096]; // Use big buffer to read to hit the bug more easily.
size_t to_read = std::min(sizeof(buf), (arg->data_count + 1 - counter) * sizeof(uint32_t));
- ASSERT_TRUE(android::base::ReadFully(arg->slave_fd, buf, to_read));
+ ASSERT_TRUE(android::base::ReadFully(arg->fd, buf, to_read));
size_t num_of_value = to_read / sizeof(uint32_t);
uint32_t* p = reinterpret_cast<uint32_t*>(buf);
while (num_of_value-- > 0) {
@@ -99,7 +99,7 @@
}
}
}
- close(arg->slave_fd);
+ close(arg->fd);
arg->finished = true;
}
@@ -114,16 +114,16 @@
constexpr uint32_t TEST_DATA_COUNT = 2000000;
// 1. Open raw pty.
- int master;
- int slave;
- ASSERT_EQ(0, openpty(&master, &slave, nullptr, nullptr, nullptr));
+ int pty;
+ int tty;
+ ASSERT_EQ(0, openpty(&pty, &tty, nullptr, nullptr, nullptr));
termios tattr;
- ASSERT_EQ(0, tcgetattr(slave, &tattr));
+ ASSERT_EQ(0, tcgetattr(tty, &tattr));
cfmakeraw(&tattr);
- ASSERT_EQ(0, tcsetattr(slave, TCSADRAIN, &tattr));
+ ASSERT_EQ(0, tcsetattr(tty, TCSADRAIN, &tattr));
- // 2. Make master thread and slave thread running on different cpus:
- // master thread uses first available cpu, and slave thread uses other cpus.
+ // 2. Make two threads running on different cpus:
+ // pty thread uses first available cpu, and tty thread uses other cpus.
PtyReader_28979140_Arg arg;
arg.main_cpu_id = -1;
for (int i = 0; i < CPU_SETSIZE; i++) {
@@ -134,9 +134,9 @@
}
ASSERT_GE(arg.main_cpu_id, 0);
- // 3. Create thread for slave reader.
+ // 3. Create thread for tty reader.
pthread_t thread;
- arg.slave_fd = slave;
+ arg.fd = tty;
arg.data_count = TEST_DATA_COUNT;
arg.matched = true;
ASSERT_EQ(0, pthread_create(&thread, nullptr,
@@ -147,7 +147,7 @@
CPU_SET(arg.main_cpu_id, &cpus);
ASSERT_EQ(0, sched_setaffinity(0, sizeof(cpu_set_t), &cpus));
- // 4. Send data to slave.
+ // 4. Send data to tty reader.
// Send a bunch of data at a time, so it is easier to catch the bug that some data isn't seen
// by the reader thread on another cpu.
uint32_t counter_buf[100];
@@ -156,11 +156,11 @@
for (size_t i = 0; i < sizeof(counter_buf) / sizeof(counter_buf[0]); ++i) {
counter_buf[i] = counter++;
}
- ASSERT_TRUE(android::base::WriteFully(master, &counter_buf, sizeof(counter_buf)));
+ ASSERT_TRUE(android::base::WriteFully(pty, &counter_buf, sizeof(counter_buf)));
ASSERT_TRUE(arg.matched) << "failed at count = " << counter;
}
ASSERT_EQ(0, pthread_join(thread, nullptr));
ASSERT_TRUE(arg.finished);
ASSERT_TRUE(arg.matched);
- close(master);
+ close(pty);
}