From 8a549cbb15e693a61483268ae25fdf9e3c8f15ff Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 14:35:14 -0500 Subject: [PATCH] Port/move some code from src/environment.cpp to src/env/mod.rs The global variables are moved (not copied) from C++ to rust and exported as extern C integers. On the rust side they are accessed only with atomic semantics but regular int access is preserved from the C++ side (until that code is also ported). --- fish-rust/src/env/mod.rs | 52 ++++++++++++++++++++++++++++++++++++++ src/builtins/read.cpp | 4 +-- src/builtins/set_color.cpp | 2 +- src/env.cpp | 7 ++--- src/env.h | 14 +++++++--- src/env_dispatch.cpp | 9 +++---- src/exec.cpp | 2 +- src/input.cpp | 2 +- src/screen.cpp | 2 +- 9 files changed, 74 insertions(+), 20 deletions(-) diff --git a/fish-rust/src/env/mod.rs b/fish-rust/src/env/mod.rs index f6787bff4..f901bad9e 100644 --- a/fish-rust/src/env/mod.rs +++ b/fish-rust/src/env/mod.rs @@ -3,6 +3,58 @@ mod environment_impl; pub mod var; +use crate::common::ToCString; pub use env_ffi::EnvStackSetResult; pub use environment::*; +use std::sync::atomic::{AtomicBool, AtomicUsize}; pub use var::*; + +/// Limit `read` to 100 MiB (bytes, not wide chars) by default. This can be overriden with the +/// `fish_read_limit` variable. +pub const DEFAULT_READ_BYTE_LIMIT: usize = 100 * 1024 * 1024; + +/// The actual `read` limit in effect, defaulting to [`DEFAULT_READ_BYTE_LIMIT`] but overridable +/// with `$fish_read_limit`. +#[no_mangle] +pub static READ_BYTE_LIMIT: AtomicUsize = AtomicUsize::new(DEFAULT_READ_BYTE_LIMIT); + +/// The curses `cur_term` TERMINAL pointer has been set up. +#[no_mangle] +pub static CURSES_INITIALIZED: AtomicBool = AtomicBool::new(false); + +/// Does the terminal have the "eat new line" glitch. +#[no_mangle] +pub static TERM_HAS_XN: AtomicBool = AtomicBool::new(false); + +mod ffi { + extern "C" { + pub fn setenv_lock( + name: *const libc::c_char, + value: *const libc::c_char, + overwrite: libc::c_int, + ); + pub fn unsetenv_lock(name: *const libc::c_char); + } +} + +/// Sets an environment variable after obtaining a lock, to try and improve the safety of +/// environment variables. +/// +/// As values could contain non-unicode characters, they must first be converted from &wstr to a +/// `CString` with [`crate::common::wcs2zstring()`]. +pub fn setenv_lock(name: S1, value: S2, overwrite: bool) { + let name = name.to_cstring(); + let value = value.to_cstring(); + unsafe { + self::ffi::setenv_lock(name.as_ptr(), value.as_ptr(), libc::c_int::from(overwrite)); + } +} + +/// Unsets an environment variable after obtaining a lock, to try and improve the safety of +/// environment variables. +pub fn unsetenv_lock(name: S) { + unsafe { + let name = name.to_cstring(); + self::ffi::unsetenv_lock(name.as_ptr()); + } +} diff --git a/src/builtins/read.cpp b/src/builtins/read.cpp index 8682f8658..50172a9d0 100644 --- a/src/builtins/read.cpp +++ b/src/builtins/read.cpp @@ -281,7 +281,7 @@ static int read_in_chunks(int fd, wcstring &buff, bool split_null, bool do_seek) return STATUS_CMD_ERROR; } finished = true; - } else if (str.size() > read_byte_limit) { + } else if (str.size() > READ_BYTE_LIMIT) { exit_res = STATUS_READ_TOO_MUCH; finished = true; } @@ -329,7 +329,7 @@ static int read_one_char_at_a_time(int fd, wcstring &buff, int nchars, bool spli } } - if (nbytes > read_byte_limit) { + if (nbytes > READ_BYTE_LIMIT) { exit_res = STATUS_READ_TOO_MUCH; break; } diff --git a/src/builtins/set_color.cpp b/src/builtins/set_color.cpp index 4299832ae..a42802174 100644 --- a/src/builtins/set_color.cpp +++ b/src/builtins/set_color.cpp @@ -104,7 +104,7 @@ static const struct woption long_options[] = {{L"background", required_argument, /// set_color builtin. maybe_t builtin_set_color(parser_t &parser, io_streams_t &streams, const wchar_t **argv) { // By the time this is called we should have initialized the curses subsystem. - assert(curses_initialized); + assert(CURSES_INITIALIZED); // Variables used for parsing the argument list. int argc = builtin_count_args(argv); diff --git a/src/env.cpp b/src/env.cpp index 9e868cb7f..fcd5c7d96 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -50,11 +50,6 @@ /// At init, we read all the environment variables from this array. extern char **environ; -bool curses_initialized = false; - -/// Does the terminal have the "eat_newline_glitch". -bool term_has_xn = false; - // static env_var_t env_var_t::new_ffi(EnvVar *ptr) { assert(ptr != nullptr && "env_var_t::new_ffi called with null pointer"); @@ -575,6 +570,7 @@ wcstring env_get_runtime_path() { static std::mutex s_setenv_lock{}; +extern "C" { void setenv_lock(const char *name, const char *value, int overwrite) { scoped_lock locker(s_setenv_lock); setenv(name, value, overwrite); @@ -584,6 +580,7 @@ void unsetenv_lock(const char *name) { scoped_lock locker(s_setenv_lock); unsetenv(name); } +} wcstring_list_ffi_t get_history_variable_text_ffi(const wcstring &fish_history_val) { wcstring_list_ffi_t out{}; diff --git a/src/env.h b/src/env.h index 8b0dc4410..c33a1a9c3 100644 --- a/src/env.h +++ b/src/env.h @@ -43,8 +43,14 @@ struct event_list_ffi_t { struct owning_null_terminated_array_t; -extern size_t read_byte_limit; -extern bool curses_initialized; +extern "C" { +extern bool CURSES_INITIALIZED; + +/// Does the terminal have the "eat_newline_glitch". +extern bool TERM_HAS_XN; + +extern size_t READ_BYTE_LIMIT; +} // Flags that may be passed as the 'mode' in env_stack_t::set() / environment_t::get(). enum : uint16_t { @@ -304,8 +310,6 @@ class env_stack_t final : public environment_t { bool get_use_posix_spawn(); -extern bool term_has_xn; // does the terminal have the "eat_newline_glitch" - /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); @@ -315,8 +319,10 @@ wcstring env_get_runtime_path(); /// A wrapper around setenv() and unsetenv() which use a lock. /// In general setenv() and getenv() are highly incompatible with threads. This makes it only /// slightly safer. +extern "C" { void setenv_lock(const char *name, const char *value, int overwrite); void unsetenv_lock(const char *name); +} /// Returns the originally inherited variables and their values. /// This is a simple key->value map and not e.g. cut into paths. diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index 58b3d3a23..bb7c7e51a 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -56,7 +56,6 @@ // Limit `read` to 100 MiB (bytes not wide chars) by default. This can be overridden by the // fish_read_limit variable. constexpr size_t DEFAULT_READ_BYTE_LIMIT = 100 * 1024 * 1024; -size_t read_byte_limit = DEFAULT_READ_BYTE_LIMIT; /// List of all locale environment variable names that might trigger (re)initializing the locale /// subsystem. These are only the variables we're possibly interested in. @@ -298,10 +297,10 @@ static void handle_read_limit_change(const environment_t &vars) { if (errno) { FLOGF(warning, "Ignoring fish_read_limit since it is not valid"); } else { - read_byte_limit = limit; + READ_BYTE_LIMIT = limit; } } else { - read_byte_limit = DEFAULT_READ_BYTE_LIMIT; + READ_BYTE_LIMIT = DEFAULT_READ_BYTE_LIMIT; } } @@ -617,12 +616,12 @@ static void init_curses(const environment_t &vars) { apply_term_hacks(vars); can_set_term_title = does_term_support_setting_title(vars); - term_has_xn = + TERM_HAS_XN = tigetflag(const_cast("xenl")) == 1; // does terminal have the eat_newline_glitch update_fish_color_support(vars); // Invalidate the cached escape sequences since they may no longer be valid. layout_cache_t::shared.clear(); - curses_initialized = true; + CURSES_INITIALIZED = true; } static constexpr const char *utf8_locales[] = { diff --git a/src/exec.cpp b/src/exec.cpp index 0b468972e..f69e2fb12 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1195,7 +1195,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, auto &ld = parser.libdata(); scoped_push is_subshell(&ld.is_subshell, true); - scoped_push read_limit(&ld.read_limit, is_subcmd ? read_byte_limit : 0); + scoped_push read_limit(&ld.read_limit, is_subcmd ? READ_BYTE_LIMIT : 0); auto prev_statuses = parser.get_last_statuses(); const cleanup_t put_back([&] { diff --git a/src/input.cpp b/src/input.cpp index 5af8202ba..7fc819920 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -854,7 +854,7 @@ std::shared_ptr input_mapping_set_t::all_mappings() { /// Create a list of terminfo mappings. static std::vector create_input_terminfo() { - assert(curses_initialized); + assert(CURSES_INITIALIZED); if (!cur_term) return {}; // setupterm() failed so we can't referency any key definitions #define TERMINFO_ADD(key) \ diff --git a/src/screen.cpp b/src/screen.cpp index 3b47df9ab..79c45fd29 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -1298,7 +1298,7 @@ void screen_t::reset_abandoning_line(int screen_width) { const_cast(exit_attribute_mode)))); // normal text ANSI escape sequence } - int newline_glitch_width = term_has_xn ? 0 : 1; + int newline_glitch_width = TERM_HAS_XN ? 0 : 1; abandon_line_string.append(screen_width - non_space_width - newline_glitch_width, L' '); }