Merge pull request #30585 from YHNdnzj/isatty-handling

various: clean up isatty() handling

src/basic/fileio.c

@@ -28,6 +28,7 @@
 #include "stdio-util.h"
 #include "string-util.h"
 #include "sync-util.h"
+#include "terminal-util.h"
 #include "tmpfile-util.h"
 
 /* The maximum size of the file we'll read in one go in read_full_file() (64M). */
@@ -1459,7 +1460,7 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
                                                      * and don't call isatty() on an invalid fd */
                                                 flags |= READ_LINE_NOT_A_TTY;
                                         else
-                                                flags |= isatty(fd) ? READ_LINE_IS_A_TTY : READ_LINE_NOT_A_TTY;
+                                                flags |= isatty_safe(fd) ? READ_LINE_IS_A_TTY : READ_LINE_NOT_A_TTY;
                                 }
                                 if (FLAGS_SET(flags, READ_LINE_IS_A_TTY))
                                         break;

src/basic/log.c

@@ -414,7 +414,7 @@ static bool check_console_fd_is_tty(void) {
                 return false;
 
         if (console_fd_is_tty < 0)
-                console_fd_is_tty = isatty(console_fd) > 0;
+                console_fd_is_tty = isatty_safe(console_fd);
 
         return console_fd_is_tty;
 }

src/basic/terminal-util.c

@@ -54,6 +54,18 @@ static volatile int cached_on_dev_null = -1;
 static volatile int cached_color_mode = _COLOR_INVALID;
 static volatile int cached_underline_enabled = -1;
 
+bool isatty_safe(int fd) {
+        assert(fd >= 0);
+
+        if (isatty(fd))
+                return true;
+
+        /* Be resilient if we're working on stdio, since they're set up by parent process. */
+        assert(errno != EBADF || IN_SET(fd, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO));
+
+        return false;
+}
+
 int chvt(int vt) {
         _cleanup_close_ int fd = -EBADF;
 
@@ -240,7 +252,7 @@ int reset_terminal_fd(int fd, bool switch_to_text) {
 
         assert(fd >= 0);
 
-        if (isatty(fd) < 1)
+        if (!isatty_safe(fd))
                 return log_debug_errno(errno, "Asked to reset a terminal that actually isn't a terminal: %m");
 
         /* We leave locked terminal attributes untouched, so that Plymouth may set whatever it wants to set,
@@ -347,7 +359,7 @@ int open_terminal(const char *name, int mode) {
                 c++;
         }
 
-        if (isatty(fd) < 1)
+        if (!isatty_safe(fd))
                 return negative_errno();
 
         return TAKE_FD(fd);
@@ -1447,38 +1459,33 @@ int vt_reset_keyboard(int fd) {
 }
 
 int vt_restore(int fd) {
+
         static const struct vt_mode mode = {
                 .mode = VT_AUTO,
         };
-        int r, q = 0;
 
-        if (isatty(fd) < 1)
+        int r, ret = 0;
+
+        assert(fd >= 0);
+
+        if (!isatty_safe(fd))
                 return log_debug_errno(errno, "Asked to restore the VT for an fd that does not refer to a terminal: %m");
 
         if (ioctl(fd, KDSETMODE, KD_TEXT) < 0)
-                q = log_debug_errno(errno, "Failed to set VT in text mode, ignoring: %m");
+                RET_GATHER(ret, log_debug_errno(errno, "Failed to set VT to text mode, ignoring: %m"));
 
         r = vt_reset_keyboard(fd);
-        if (r < 0) {
+        if (r < 0)
-                log_debug_errno(r, "Failed to reset keyboard mode, ignoring: %m");
+                RET_GATHER(ret, log_debug_errno(r, "Failed to reset keyboard mode, ignoring: %m"));
-                if (q >= 0)
-                        q = r;
-        }
 
-        if (ioctl(fd, VT_SETMODE, &mode) < 0) {
+        if (ioctl(fd, VT_SETMODE, &mode) < 0)
-                log_debug_errno(errno, "Failed to set VT_AUTO mode, ignoring: %m");
+                RET_GATHER(ret, log_debug_errno(errno, "Failed to set VT_AUTO mode, ignoring: %m"));
-                if (q >= 0)
-                        q = -errno;
-        }
 
         r = fchmod_and_chown(fd, TTY_MODE, 0, GID_INVALID);
-        if (r < 0) {
+        if (r < 0)
-                log_debug_errno(r, "Failed to chmod()/chown() VT, ignoring: %m");
+                RET_GATHER(ret, log_debug_errno(r, "Failed to chmod()/chown() VT, ignoring: %m"));
-                if (q >= 0)
-                        q = r;
-        }
 
-        return q;
+        return ret;
 }
 
 int vt_release(int fd, bool restore) {
@@ -1488,7 +1495,7 @@ int vt_release(int fd, bool restore) {
          * sent by the kernel and optionally reset the VT in text and auto
          * VT-switching modes. */
 
-        if (isatty(fd) < 1)
+        if (!isatty_safe(fd))
                 return log_debug_errno(errno, "Asked to release the VT for an fd that does not refer to a terminal: %m");
 
         if (ioctl(fd, VT_RELDISP, 1) < 0)
@@ -1698,7 +1705,7 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret
         if (!colors_enabled())
                 return -EOPNOTSUPP;
 
-        if (isatty(STDOUT_FILENO) < 1 || isatty(STDIN_FILENO) < 1)
+        if (!isatty(STDIN_FILENO) || !isatty(STDOUT_FILENO))
                 return -EOPNOTSUPP;
 
         if (streq_ptr(getenv("TERM"), "linux")) {

src/basic/terminal-util.h

@@ -86,6 +86,8 @@
 /* Set cursor to top left corner and clear screen */
 #define ANSI_HOME_CLEAR "\x1B[H\x1B[2J"
 
+bool isatty_safe(int fd);
+
 int reset_terminal_fd(int fd, bool switch_to_text);
 int reset_terminal(const char *name);
 int set_terminal_cursor_position(int fd, unsigned int row, unsigned int column);

src/busctl/busctl.c

@@ -1369,7 +1369,7 @@ static int verb_capture(int argc, char **argv, void *userdata) {
                 "busctl (systemd) " STRINGIFY(PROJECT_VERSION) " (Git " GIT_VERSION ")";
         int r;
 
-        if (isatty(fileno(stdout)) > 0)
+        if (isatty(STDOUT_FILENO))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                        "Refusing to write message data to console, please redirect output to a file.");
 

src/core/exec-invoke.c

@@ -670,12 +670,8 @@ static int chown_terminal(int fd, uid_t uid) {
         assert(fd >= 0);
 
         /* Before we chown/chmod the TTY, let's ensure this is actually a tty */
-        if (isatty(fd) < 1) {
+        if (!isatty_safe(fd))
-                if (IN_SET(errno, EINVAL, ENOTTY))
+                return 0;
-                        return 0; /* not a tty */
-
-                return -errno;
-        }
 
         /* This might fail. What matters are the results. */
         r = fchmod_and_chown(fd, TTY_MODE, uid, GID_INVALID);

src/core/execute.c

@@ -147,7 +147,7 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p)
 
         const char *path = exec_context_tty_path(context);
 
-        if (p && p->stdin_fd >= 0 && isatty(p->stdin_fd))
+        if (p && p->stdin_fd >= 0 && isatty_safe(p->stdin_fd))
                 fd = p->stdin_fd;
         else if (path && (context->tty_path || is_terminal_input(context->std_input) ||
                         is_terminal_output(context->std_output) || is_terminal_output(context->std_error))) {

src/creds/creds.c

@@ -315,7 +315,7 @@ static int print_newline(FILE *f, const char *data, size_t l) {
 
         /* Don't bother unless this is a tty */
         fd = fileno(f);
-        if (fd >= 0 && isatty(fd) <= 0)
+        if (fd >= 0 && !isatty_safe(fd))
                 return 0;
 
         if (fputc('\n', f) != '\n')

src/getty-generator/getty-generator.c

@@ -94,9 +94,8 @@ static int verify_tty(const char *name) {
         if (fd < 0)
                 return -errno;
 
-        errno = 0;
+        if (!isatty_safe(fd))
-        if (isatty(fd) <= 0)
+                return -errno;
-                return errno_or_else(EIO);
 
         return 0;
 }

src/journal/journald.c

@@ -13,6 +13,7 @@
 #include "main-func.h"
 #include "process-util.h"
 #include "sigbus.h"
+#include "terminal-util.h"
 
 static int run(int argc, char *argv[]) {
         _cleanup_(server_freep) Server *s = NULL;
@@ -35,7 +36,7 @@ static int run(int argc, char *argv[]) {
                  * daemon when it comes to logging hence LOG_TARGET_AUTO won't do the right thing for
                  * us. Hence explicitly log to the console if we're started from a console or to kmsg
                  * otherwise. */
-                log_target = isatty(STDERR_FILENO) > 0 ? LOG_TARGET_CONSOLE : LOG_TARGET_KMSG;
+                log_target = isatty(STDERR_FILENO) ? LOG_TARGET_CONSOLE : LOG_TARGET_KMSG;
 
                 log_set_prohibit_ipc(true); /* better safe than sorry */
                 log_set_target(log_target);

src/nspawn/nspawn.c

@@ -289,7 +289,7 @@ static int handle_arg_console(const char *arg) {
         else if (streq(arg, "passive"))
                 arg_console_mode = CONSOLE_PASSIVE;
         else if (streq(arg, "pipe")) {
-                if (isatty(STDIN_FILENO) > 0 && isatty(STDOUT_FILENO) > 0)
+                if (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO))
                         log_full(arg_quiet ? LOG_DEBUG : LOG_NOTICE,
                                  "Console mode 'pipe' selected, but standard input/output are connected to an interactive TTY. "
                                  "Most likely you want to use 'interactive' console mode for proper interactivity and shell job control. "
@@ -297,7 +297,7 @@ static int handle_arg_console(const char *arg) {
 
                 arg_console_mode = CONSOLE_PIPE;
         } else if (streq(arg, "autopipe")) {
-                if (isatty(STDIN_FILENO) > 0 && isatty(STDOUT_FILENO) > 0)
+                if (isatty(STDIN_FILENO) && isatty(STDOUT_FILENO))
                         arg_console_mode = CONSOLE_INTERACTIVE;
                 else
                         arg_console_mode = CONSOLE_PIPE;
@@ -5766,9 +5766,8 @@ static int run(int argc, char *argv[]) {
                 goto finish;
 
         if (arg_console_mode < 0)
-                arg_console_mode =
+                arg_console_mode = isatty(STDIN_FILENO) && isatty(STDOUT_FILENO) ?
-                        isatty(STDIN_FILENO) > 0 &&
+                                   CONSOLE_INTERACTIVE : CONSOLE_READ_ONLY;
-                        isatty(STDOUT_FILENO) > 0 ? CONSOLE_INTERACTIVE : CONSOLE_READ_ONLY;
 
         if (arg_console_mode == CONSOLE_PIPE) /* if we pass STDERR on to the container, don't add our own logs into it too */
                 arg_quiet = true;

src/shared/wall.c

@@ -30,8 +30,9 @@ static int write_to_terminal(const char *tty, const char *message) {
         fd = open(tty, O_WRONLY|O_NONBLOCK|O_NOCTTY|O_CLOEXEC);
         if (fd < 0)
                 return -errno;
-        if (!isatty(fd))
+
-                return -ENOTTY;
+        if (!isatty_safe(fd))
+                return -errno;
 
         return loop_write_full(fd, message, SIZE_MAX, TIMEOUT_USEC);
 }

src/storagetm/storagetm.c

@@ -1043,7 +1043,7 @@ static int on_display_refresh(sd_event_source *s, uint64_t usec, void *userdata)
 
         c->display_refresh_scheduled = false;
 
-        if (isatty(STDERR_FILENO) > 0)
+        if (isatty(STDERR_FILENO))
                 fputs(ANSI_HOME_CLEAR, stderr);
 
         /* If we have both IPv4 and IPv6, we display IPv4 info via Plymouth, since it doesn't have much
@@ -1224,7 +1224,7 @@ static int run(int argc, char* argv[]) {
         if (r < 0)
                 return log_error_errno(r, "Failed to subscribe to RTM_DELADDR events: %m");
 
-        if (isatty(0) > 0)
+        if (isatty(STDIN_FILENO))
                 log_info("Hit Ctrl-C to exit target mode.");
 
         _unused_ _cleanup_(notify_on_cleanup) const char *notify_message =