commit 65afc844c0176313c8f97c87d3c20b6671aa73ba
author Ben Clayton <bclayton@google.com> Wed Mar 13 17:57:29 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 13 17:57:29 2024 +0000
tree 4b5b1c8463986f0fde1d3eecd1b5cda9366c9464
parent f9a5b712d89d18e969d08e1f7b90fd0c6197b56b

[tint][utils] Fixes for TerminalIsDark()

Only query the background color if stdin isatty, and tcgetattr() / tcsetattr() succeed. Otherwise the terminal will echo the terminal colors.

Poll stdin, to ensure it doesn't block indefinitely.

Fixed: tint:2199
Fixed: tint:2196
Change-Id: Id94374fed3d89993fd5262c4089c17bb01f590d6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/178960
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/utils/system/terminal_posix.cc

diff --git a/src/tint/utils/system/terminal_posix.cc b/src/tint/utils/system/terminal_posix.cc
index fb2d93f..e820774 100644
--- a/src/tint/utils/system/terminal_posix.cc
+++ b/src/tint/utils/system/terminal_posix.cc
@@ -39,6 +39,7 @@
 #include <utility>
 
 #include "src/tint/utils/containers/vector.h"
+#include "src/tint/utils/macros/compiler.h"
 #include "src/tint/utils/macros/defer.h"
 #include "src/tint/utils/system/env.h"
 #include "src/tint/utils/system/terminal.h"
@@ -47,7 +48,8 @@
 namespace {
 
 std::optional<bool> TerminalIsDarkImpl(FILE* out) {
-    if (!TerminalSupportsColors(out)) {
+    // Check the terminal can be queried, and supports colors.
+    if (!isatty(STDIN_FILENO) || !TerminalSupportsColors(out)) {
         return std::nullopt;
     }
 
@@ -59,17 +61,22 @@
 
     // Store the current attributes for 'out', restore it before returning
     termios original_state{};
-    tcgetattr(out_fd, &original_state);
+    if (tcgetattr(out_fd, &original_state) != 0) {
+        return std::nullopt;
+    }
     TINT_DEFER(tcsetattr(out_fd, TCSADRAIN, &original_state));
 
     // Prevent echoing.
     termios state = original_state;
     state.c_lflag &= ~static_cast<tcflag_t>(ECHO | ICANON);
-    tcsetattr(out_fd, TCSADRAIN, &state);
+    if (tcsetattr(out_fd, TCSADRAIN, &state) != 0) {
+        return std::nullopt;
+    }
 
     // Emit the device control escape sequence to query the terminal colors.
     static constexpr std::string_view kQuery = "\033]11;?\033\\";
     fwrite(kQuery.data(), 1, kQuery.length(), out);
+    fflush(out);
 
     // Timeout for attempting to read the response.
     static constexpr auto kTimeout = std::chrono::milliseconds(300);
@@ -77,6 +84,22 @@
     // Record the start time.
     auto start = std::chrono::steady_clock::now();
 
+    // Returns true if there's data available on stdin, or false if no data was available after
+    // 100ms.
+    auto poll_stdin = [] {
+        // Note: These macros introduce identifiers that start with `__`.
+        TINT_BEGIN_DISABLE_WARNING(RESERVED_IDENTIFIER);
+        fd_set rfds{};
+        FD_ZERO(&rfds);
+        FD_SET(STDIN_FILENO, &rfds);
+        timeval tv{};
+        tv.tv_sec = 0;
+        tv.tv_usec = 100'000;
+        int res = select(STDIN_FILENO + 1, &rfds, nullptr, nullptr, &tv);
+        return res > 0 && FD_ISSET(STDIN_FILENO, &rfds);
+        TINT_END_DISABLE_WARNING(RESERVED_IDENTIFIER);
+    };
+
     // Helpers for parsing the response.
     Vector<char, 8> peek;
     auto read = [&]() -> std::optional<char> {
@@ -84,6 +107,10 @@
             return peek.Pop();
         }
         while ((std::chrono::steady_clock::now() - start) < kTimeout) {
+            if (!poll_stdin()) {
+                return std::nullopt;
+            }
+
             char c;
             if (fread(&c, 1, 1, stdin) == 1) {
                 return c;
@@ -163,7 +190,7 @@
             return std::nullopt;
     }
 
-    if (!match("\x07") && !match("\x1b\x5c")) {
+    if (!match("\x07") && !match("\x1b")) {
         return std::nullopt;
     }