Turn a lot of common.h variables into getter functions

Improves thread safety.

src/builtin_set.cpp

@@ -494,7 +494,7 @@ static int builtin_set_list(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
                 streams.out.append(L" ");
                 streams.out.append(val);
 
-                if (shorten) streams.out.push_back(ellipsis_char);
+                if (shorten) streams.out.push_back(get_ellipsis_char());
             }
         }
 

src/common.cpp

@@ -53,6 +53,7 @@
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "future_feature_flags.h"
+#include "global_safety.h"
 #include "proc.h"
 #include "wildcard.h"
 #include "wutil.h"  // IWYU pragma: keep
@@ -64,19 +65,32 @@ struct termios shell_modes;
 /// This allows us to determine if we're running on the main thread
 static std::atomic<size_t> thread_id { 0 };
 /// This allows us to notice when we've forked.
-static bool is_forked_proc = false;
+static relaxed_atomic_bool_t is_forked_proc{false};
 /// This allows us to bypass the main thread checks
-static bool thread_asserts_cfg_for_testing = false;
+static relaxed_atomic_bool_t thread_asserts_cfg_for_testing{false};
+
+static relaxed_atomic_t<wchar_t> ellipsis_char;
+wchar_t get_ellipsis_char() { return ellipsis_char; }
+
+static relaxed_atomic_t<const wchar_t *> ellipsis_str;
+const wchar_t *get_ellipsis_str() { return ellipsis_str; }
+
+static relaxed_atomic_t<const wchar_t *> omitted_newline_str;
+const wchar_t *get_omitted_newline_str() { return omitted_newline_str; }
+
+static relaxed_atomic_t<int> omitted_newline_width;
+int get_omitted_newline_width() { return omitted_newline_width; }
+
+static relaxed_atomic_t<wchar_t> obfuscation_read_char;
+wchar_t get_obfuscation_read_char() { return obfuscation_read_char; }
 
-wchar_t ellipsis_char;
-const wchar_t *ellipsis_str = nullptr;
-const wchar_t *omitted_newline_str;
-int omitted_newline_width;
-wchar_t obfuscation_read_char;
 bool g_profiling_active = false;
 const wchar_t *program_name;
 std::atomic<int> debug_level{1};  // default maximum debug output level (errors and warnings)
-int debug_stack_frames = 0;  // default number of stack frames to show on debug() calls
+
+static relaxed_atomic_t<int> debug_stack_frames{0};
+void set_debug_stack_frames(int v) { debug_stack_frames = v; }
+int get_debug_stack_frames() { return debug_stack_frames; }
 
 /// Be able to restore the term's foreground process group.
 /// This is set during startup and not modified after.

src/common.h

@@ -176,20 +176,22 @@ extern struct termios shell_modes;
 
 /// The character to use where the text has been truncated. Is an ellipsis on unicode system and a $
 /// on other systems.
-extern wchar_t ellipsis_char;
+wchar_t get_ellipsis_char();
+
 /// The character or string to use where text has been truncated (ellipsis if possible, otherwise
 /// ...)
-extern const wchar_t *ellipsis_str;
+const wchar_t *get_ellipsis_str();
 
 /// Character representing an omitted newline at the end of text.
-extern const wchar_t *omitted_newline_str;
+const wchar_t *get_omitted_newline_str();
-extern int omitted_newline_width;
+int get_omitted_newline_width();
 
 /// Character used for the silent mode of the read command
-extern wchar_t obfuscation_read_char;
+wchar_t get_obfuscation_read_char();
 
 /// How many stack frames to show when a debug() call is made.
-extern int debug_stack_frames;
+int get_debug_stack_frames();
+void set_debug_stack_frames(int);
 
 /// Profiling flag. True if commands should be profiled.
 extern bool g_profiling_active;

src/fish.cpp

@@ -337,7 +337,7 @@ static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) {
                 tmp = strtol(optarg, &end, 10);
 
                 if (tmp > 0 && tmp <= 128 && !*end && !errno) {
-                    debug_stack_frames = (int)tmp;
+                    set_debug_stack_frames((int)tmp);
                 } else {
                     std::fwprintf(stderr, _(L"Invalid value '%s' for debug-stack-frames flag"), optarg);
                     exit(1);

src/fish_indent.cpp

@@ -575,7 +575,7 @@ int main(int argc, char *argv[]) {
                 tmp = strtol(optarg, &end, 10);
 
                 if (tmp > 0 && tmp <= 128 && !*end && !errno) {
-                    debug_stack_frames = (int)tmp;
+                    set_debug_stack_frames((int)tmp);
                 } else {
                     std::fwprintf(stderr, _(L"Invalid value '%s' for debug-stack-frames flag"), optarg);
                     exit(1);

src/fish_key_reader.cpp

@@ -324,7 +324,7 @@ static bool parse_debug_frames_flag() {
     char *end;
     long tmp = strtol(optarg, &end, 10);
     if (tmp > 0 && tmp <= 128 && !*end && !errno) {
-        debug_stack_frames = (int)tmp;
+        set_debug_stack_frames((int)tmp);
     } else {
         std::fwprintf(stderr, _(L"Invalid value '%s' for debug-stack-frames flag\n"), optarg);
         return false;

src/fish_tests.cpp

@@ -2047,6 +2047,7 @@ struct pager_layout_testcase_t {
             wcstring expected = this->expected;
 
             // hack: handle the case where ellipsis is not L'\x2026'
+            wchar_t ellipsis_char = get_ellipsis_char();
             if (ellipsis_char != L'\x2026') {
                 std::replace(expected.begin(), expected.end(), L'\x2026', ellipsis_char);
             }

src/global_safety.h

@@ -6,6 +6,7 @@
 
 #include "common.h"
 
+#include <atomic>
 #include <cassert>
 
 // fish is multithreaded. Global (which includes function and file-level statics) when used naively
@@ -70,4 +71,20 @@ class latch_t : detail::fixed_t {
     }
 };
 
+/// An atomic type that always use relaxed reads.
+template <typename T>
+class relaxed_atomic_t {
+    std::atomic<T> value_{};
+
+   public:
+    relaxed_atomic_t() = default;
+    relaxed_atomic_t(T value) : value_(value) {}
+
+    operator T() const { return value_.load(std::memory_order_relaxed); }
+
+    void operator=(T v) { return value_.store(v, std::memory_order_relaxed); }
+};
+
+using relaxed_atomic_bool_t = relaxed_atomic_t<bool>;
+
 #endif

src/pager.cpp

@@ -89,9 +89,10 @@ static size_t print_max(const wcstring &str, highlight_spec_t color, size_t max,
 
         if (width_c > remaining) break;
 
+        wchar_t ellipsis = get_ellipsis_char();
         if ((width_c == remaining) && (has_more || i + 1 < str.size())) {
-            line->append(ellipsis_char, color);
+            line->append(ellipsis, color);
-            int ellipsis_width = fish_wcwidth(ellipsis_char);
+            int ellipsis_width = fish_wcwidth(ellipsis);
             remaining -= std::min(remaining, size_t(ellipsis_width));
             break;
         }
@@ -493,7 +494,7 @@ bool pager_t::completion_try_print(size_t cols, const wcstring &prefix, const co
     wcstring progress_text;
     assert(rendering->remaining_to_disclose != 1);
     if (rendering->remaining_to_disclose > 1) {
-        progress_text = format_string(_(L"%lsand %lu more rows"), ellipsis_str,
+        progress_text = format_string(_(L"%lsand %lu more rows"), get_ellipsis_str(),
                                       (unsigned long)rendering->remaining_to_disclose);
     } else if (start_row > 0 || stop_row < row_count) {
         // We have a scrollable interface. The +1 here is because we are zero indexed, but want

src/parse_execution.cpp

@@ -700,7 +700,7 @@ parse_execution_result_t parse_execution_context_t::handle_command_not_found(
             // Looks like a command.
             this->report_error(statement, ERROR_BAD_EQUALS_IN_COMMAND5, argument.c_str(),
                                name_str.c_str(), val_str.c_str(), argument.c_str(),
-                               ellipsis_str);
+                               get_ellipsis_str());
         } else {
             wcstring assigned_val = reconstruct_orig_str(val_str);
             this->report_error(statement, ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, name_str.c_str(),

src/parse_util.cpp

@@ -794,6 +794,7 @@ static wcstring truncate_string(const wcstring &str) {
     wcstring result(str, 0, max_len);
     if (str.size() > max_len) {
         // Truncate!
+        wchar_t ellipsis_char = get_ellipsis_char();
         if (ellipsis_char == L'\x2026') {
             result.at(max_len - 1) = ellipsis_char;
         } else {

src/proc.cpp

@@ -410,6 +410,7 @@ static wcstring truncate_command(const wcstring &cmd) {
     }
 
     // Truncation required.
+    const wchar_t *ellipsis_str = get_ellipsis_str();
     const size_t ellipsis_length = std::wcslen(ellipsis_str);  // no need for wcwidth
     size_t trunc_length = max_len - ellipsis_length;
     // Eat trailing whitespace.

src/reader.cpp

@@ -610,7 +610,7 @@ void reader_data_t::repaint() {
 
     wcstring full_line;
     if (silent) {
-        full_line = wcstring(cmd_line->text.length(), obfuscation_read_char);
+        full_line = wcstring(cmd_line->text.length(), get_obfuscation_read_char());
     } else {
         // Combine the command and autosuggestion into one string.
         full_line = combine_command_and_autosuggestion(cmd_line->text, autosuggestion);
@@ -1633,7 +1633,7 @@ bool reader_data_t::handle_completions(const std::vector<completion_t> &comp,
         prefix.append(el->text, prefix_start, len);
     } else {
         // Append just the end of the string.
-        prefix = wcstring(&ellipsis_char, 1);
+        prefix = wcstring{get_ellipsis_char()};
         prefix.append(el->text, prefix_start + len - PREFIX_MAX_LEN, PREFIX_MAX_LEN);
     }
 

src/screen.cpp

@@ -918,7 +918,7 @@ static screen_layout_t compute_layout(screen_t *s, size_t screen_width,
                 size_t truncation_offset = truncation_offset_for_width(
                     autosuggest_truncated_widths, available_autosuggest_space - 2);
                 result.autosuggestion = wcstring(autosuggestion, truncation_offset);
-                result.autosuggestion.push_back(ellipsis_char);
+                result.autosuggestion.push_back(get_ellipsis_char());
             }
             done = true;
         }
@@ -1102,7 +1102,7 @@ void s_reset(screen_t *s, screen_reset_mode_t mode) {
 
         // Don't need to check for fish_wcwidth errors; this is done when setting up
         // omitted_newline_char in common.cpp.
-        int non_space_width = omitted_newline_width;
+        int non_space_width = get_omitted_newline_width();
         // We do `>` rather than `>=` because the code below might require one extra space.
         if (screen_width > non_space_width) {
             bool justgrey = true;
@@ -1129,7 +1129,7 @@ void s_reset(screen_t *s, screen_reset_mode_t mode) {
                 }
             }
 
-            abandon_line_string.append(omitted_newline_str);
+            abandon_line_string.append(get_omitted_newline_str());
 
             if (cur_term && exit_attribute_mode) {
                 abandon_line_string.append(
@@ -1141,7 +1141,7 @@ void s_reset(screen_t *s, screen_reset_mode_t mode) {
         }
 
         abandon_line_string.push_back(L'\r');
-        abandon_line_string.append(omitted_newline_str);
+        abandon_line_string.append(get_omitted_newline_str());
         // Now we are certainly on a new line. But we may have dropped the omitted newline char on
         // it. So append enough spaces to overwrite the omitted newline char, and then clear all the
         // spaces from the new line.

src/wcstringutil.cpp

@@ -39,10 +39,11 @@ wcstring truncate(const wcstring &input, int max_len, ellipsis_type etype) {
         return input.substr(0, max_len);
     }
     if (etype == ellipsis_type::Prettiest) {
+        const wchar_t *ellipsis_str = get_ellipsis_str();
         return input.substr(0, max_len - std::wcslen(ellipsis_str)).append(ellipsis_str);
     }
     wcstring output = input.substr(0, max_len - 1);
-    output.push_back(ellipsis_char);
+    output.push_back(get_ellipsis_char());
     return output;
 }