Skip to content

Commit 4049879

Browse files
vtjnashsantigimeno
vtjnash
authored and
santigimeno
committed
stream: autodetect direction
Previously, we required the user to specify the expected read/write flags for a pipe or tty. But we've already been asking the OS to tell us what they actually are (fcntl F_GETFL), so we can hopefully just use that information directly. Fixes: #1936 PR-URL: #1964 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
1 parent 956bf6b commit 4049879

11 files changed

Lines changed: 110 additions & 38 deletions

File tree

‎docs/code/tty-gravity/main.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void update(uv_timer_t *req) {
3232
int main() {
3333
loop = uv_default_loop();
3434

35-
uv_tty_init(loop, &tty, 1, 0);
35+
uv_tty_init(loop, &tty, STDOUT_FILENO, 0);
3636
uv_tty_set_mode(&tty, 0);
3737

3838
if (uv_tty_get_winsize(&tty, &width, &height)) {

‎docs/code/tty/main.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ uv_tty_t tty;
88
int main() {
99
loop = uv_default_loop();
1010

11-
uv_tty_init(loop, &tty, 1, 0);
11+
uv_tty_init(loop, &tty, STDOUT_FILENO, 0);
1212
uv_tty_set_mode(&tty, UV_TTY_MODE_NORMAL);
1313

1414
if (uv_guess_handle(1) == UV_TTY) {

‎docs/src/guide/utilities.rst‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,10 @@ terminal information.
370370
The first thing to do is to initialize a ``uv_tty_t`` with the file descriptor
371371
it reads/writes from. This is achieved with::
372372

373-
int uv_tty_init(uv_loop_t*, uv_tty_t*, uv_file fd, int readable)
373+
int uv_tty_init(uv_loop_t*, uv_tty_t*, uv_file fd, int unused)
374374

375-
Set ``readable`` to true if you plan to use ``uv_read_start()`` on the stream.
375+
The ``unused`` parameter is now auto-detected and ignored. It previously needed
376+
to be set to use ``uv_read_start()`` on the stream.
376377

377378
It is then best to use ``uv_tty_set_mode`` to set the mode to *normal*
378379
which enables most TTY formatting, flow-control and other settings. Other_ modes
@@ -423,6 +424,9 @@ can try `ncurses`_.
423424

424425
.. _ncurses: http://www.gnu.org/software/ncurses/ncurses.html
425426

427+
.. versionchanged:: 1.23.1: the `readable` parameter is now unused and ignored.
428+
The appropriate value will now be auto-detected from the kernel.
429+
426430
----
427431

428432
.. [#] I was first introduced to the term baton in this context, in Konstantin

‎docs/src/tty.rst‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ N/A
4646
API
4747
---
4848

49-
.. c:function:: int uv_tty_init(uv_loop_t* loop, uv_tty_t* handle, uv_file fd, int readable)
49+
.. c:function:: int uv_tty_init(uv_loop_t* loop, uv_tty_t* handle, uv_file fd, int unused)
5050
5151
Initialize a new TTY stream with the given file descriptor. Usually the
5252
file descriptor will be:
@@ -55,9 +55,6 @@ API
5555
* 1 = stdout
5656
* 2 = stderr
5757
58-
`readable`, specifies if you plan on calling :c:func:`uv_read_start` with
59-
this stream. stdin is readable, stdout is not.
60-
6158
On Unix this function will determine the path of the fd of the terminal
6259
using :man:`ttyname_r(3)`, open it, and use it if the passed file descriptor
6360
refers to a TTY. This lets libuv put the tty in non-blocking mode without
@@ -67,8 +64,10 @@ API
6764
ioctl TIOCGPTN or TIOCPTYGNAME, for instance OpenBSD and Solaris.
6865
6966
.. note::
70-
If reopening the TTY fails, libuv falls back to blocking writes for
71-
non-readable TTY streams.
67+
If reopening the TTY fails, libuv falls back to blocking writes.
68+
69+
.. versionchanged:: 1.23.1: the `readable` parameter is now unused and ignored.
70+
The correct value will now be auto-detected from the kernel.
7271
7372
.. versionchanged:: 1.9.0: the path of the TTY is determined by
7473
:man:`ttyname_r(3)`. In earlier versions libuv opened

‎src/unix/pipe.c‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,21 @@ void uv__pipe_close(uv_pipe_t* handle) {
132132

133133

134134
int uv_pipe_open(uv_pipe_t* handle, uv_file fd) {
135+
int flags;
136+
int mode;
135137
int err;
138+
flags = 0;
136139

137140
if (uv__fd_exists(handle->loop, fd))
138141
return UV_EEXIST;
139142

143+
do
144+
mode = fcntl(fd, F_GETFL);
145+
while (mode == -1 && errno == EINTR);
146+
147+
if (mode == -1)
148+
return UV__ERR(errno); /* according to docs, must be EBADF */
149+
140150
err = uv__nonblock(fd, 1);
141151
if (err)
142152
return err;
@@ -147,9 +157,13 @@ int uv_pipe_open(uv_pipe_t* handle, uv_file fd) {
147157
return err;
148158
#endif /* defined(__APPLE__) */
149159

150-
return uv__stream_open((uv_stream_t*)handle,
151-
fd,
152-
UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
160+
mode &= O_ACCMODE;
161+
if (mode != O_WRONLY)
162+
flags |= UV_HANDLE_READABLE;
163+
if (mode != O_RDONLY)
164+
flags |= UV_HANDLE_WRITABLE;
165+
166+
return uv__stream_open((uv_stream_t*)handle, fd, flags);
153167
}
154168

155169

‎src/unix/stream.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1676,6 +1676,7 @@ void uv__stream_close(uv_stream_t* handle) {
16761676
uv__io_close(handle->loop, &handle->io_watcher);
16771677
uv_read_stop(handle);
16781678
uv__handle_stop(handle);
1679+
handle->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
16791680

16801681
if (handle->io_watcher.fd != -1) {
16811682
/* Don't close stdio file descriptors. Nothing good comes from it. */

‎src/unix/tty.c‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,15 @@ static int uv__tty_is_slave(const int fd) {
9292
return result;
9393
}
9494

95-
int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
95+
int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int unused) {
9696
uv_handle_type type;
9797
int flags;
9898
int newfd;
9999
int r;
100100
int saved_flags;
101+
int mode;
101102
char path[256];
103+
(void)unused; /* deprecated parameter is no longer needed */
102104

103105
/* File descriptors that refer to files cannot be monitored with epoll.
104106
* That restriction also applies to character devices like /dev/random
@@ -111,6 +113,15 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
111113
flags = 0;
112114
newfd = -1;
113115

116+
/* Save the fd flags in case we need to restore them due to an error. */
117+
do
118+
saved_flags = fcntl(fd, F_GETFL);
119+
while (saved_flags == -1 && errno == EINTR);
120+
121+
if (saved_flags == -1)
122+
return UV__ERR(errno);
123+
mode = saved_flags & O_ACCMODE;
124+
114125
/* Reopen the file descriptor when it refers to a tty. This lets us put the
115126
* tty in non-blocking mode without affecting other processes that share it
116127
* with us.
@@ -128,13 +139,13 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
128139
* slave device.
129140
*/
130141
if (uv__tty_is_slave(fd) && ttyname_r(fd, path, sizeof(path)) == 0)
131-
r = uv__open_cloexec(path, O_RDWR);
142+
r = uv__open_cloexec(path, mode);
132143
else
133144
r = -1;
134145

135146
if (r < 0) {
136147
/* fallback to using blocking writes */
137-
if (!readable)
148+
if (mode != O_RDONLY)
138149
flags |= UV_HANDLE_BLOCKING_WRITES;
139150
goto skip;
140151
}
@@ -154,22 +165,6 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
154165
fd = newfd;
155166
}
156167

157-
#if defined(__APPLE__)
158-
/* Save the fd flags in case we need to restore them due to an error. */
159-
do
160-
saved_flags = fcntl(fd, F_GETFL);
161-
while (saved_flags == -1 && errno == EINTR);
162-
163-
if (saved_flags == -1) {
164-
if (newfd != -1)
165-
uv__close(newfd);
166-
return UV__ERR(errno);
167-
}
168-
#endif
169-
170-
/* Pacify the compiler. */
171-
(void) &saved_flags;
172-
173168
skip:
174169
uv__stream_init(loop, (uv_stream_t*) tty, UV_TTY);
175170

@@ -194,9 +189,9 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
194189
}
195190
#endif
196191

197-
if (readable)
192+
if (mode != O_WRONLY)
198193
flags |= UV_HANDLE_READABLE;
199-
else
194+
if (mode != O_RDONLY)
200195
flags |= UV_HANDLE_WRITABLE;
201196

202197
uv__stream_open((uv_stream_t*) tty, fd, flags);

‎src/win/tty.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,12 @@ void uv_console_init(void) {
172172
}
173173

174174

175-
int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int readable) {
175+
int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int unused) {
176+
BOOL readable;
177+
DWORD NumberOfEvents;
176178
HANDLE handle;
177179
CONSOLE_SCREEN_BUFFER_INFO screen_buffer_info;
180+
(void)unused;
178181

179182
uv__once_init();
180183
handle = (HANDLE) uv__get_osfhandle(fd);
@@ -199,6 +202,7 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int readable) {
199202
fd = -1;
200203
}
201204

205+
readable = GetNumberOfConsoleInputEvents(handle, &NumberOfEvents);
202206
if (!readable) {
203207
/* Obtain the screen buffer info with the output handle. */
204208
if (!GetConsoleScreenBufferInfo(handle, &screen_buffer_info)) {

‎test/test-handle-fileno.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ static int get_tty_fd(void) {
2727
/* Make sure we have an FD that refers to a tty */
2828
#ifdef _WIN32
2929
HANDLE handle;
30-
handle = CreateFileA("conout$",
30+
handle = CreateFileA("conin$",
3131
GENERIC_READ | GENERIC_WRITE,
3232
FILE_SHARE_READ | FILE_SHARE_WRITE,
3333
NULL,
@@ -107,11 +107,15 @@ TEST_IMPL(handle_fileno) {
107107
} else {
108108
r = uv_tty_init(loop, &tty, tty_fd, 0);
109109
ASSERT(r == 0);
110+
ASSERT(uv_is_readable((uv_stream_t*) &tty));
111+
ASSERT(!uv_is_writable((uv_stream_t*) &tty));
110112
r = uv_fileno((uv_handle_t*) &tty, &fd);
111113
ASSERT(r == 0);
112114
uv_close((uv_handle_t*) &tty, NULL);
113115
r = uv_fileno((uv_handle_t*) &tty, &fd);
114116
ASSERT(r == UV_EBADF);
117+
ASSERT(!uv_is_readable((uv_stream_t*) &tty));
118+
ASSERT(!uv_is_writable((uv_stream_t*) &tty));
115119
}
116120

117121
uv_run(loop, UV_RUN_DEFAULT);

‎test/test-spawn.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,6 +1733,7 @@ TEST_IMPL(spawn_inherit_streams) {
17331733
uv_buf_t buf;
17341734
unsigned int i;
17351735
int r;
1736+
int bidir;
17361737
uv_write_t write_req;
17371738
uv_loop_t* loop;
17381739

@@ -1751,6 +1752,15 @@ TEST_IMPL(spawn_inherit_streams) {
17511752
ASSERT(uv_pipe_open(&pipe_stdout_child, fds_stdout[1]) == 0);
17521753
ASSERT(uv_pipe_open(&pipe_stdin_parent, fds_stdin[1]) == 0);
17531754
ASSERT(uv_pipe_open(&pipe_stdout_parent, fds_stdout[0]) == 0);
1755+
ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdin_child));
1756+
ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdout_child));
1757+
ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdin_parent));
1758+
ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdout_parent));
1759+
/* Some systems (SVR4) open a bidirectional pipe, most don't. */
1760+
bidir = uv_is_writable((uv_stream_t*) &pipe_stdin_child);
1761+
ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdout_child) == bidir);
1762+
ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdin_parent) == bidir);
1763+
ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdout_parent) == bidir);
17541764

17551765
child_stdio[0].flags = UV_INHERIT_STREAM;
17561766
child_stdio[0].data.stream = (uv_stream_t *)&pipe_stdin_child;

0 commit comments

Comments
 (0)