From cb3ab80cabeb5c19b46cb7b114d723a20b3da7ad Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Mon, 8 Feb 2021 15:09:10 -0600 Subject: [PATCH] Use const_strlen in a few different places This may slightly improve performance by allowing the compiler greater visibility into what is happing on top of not executing at runtime in some hot paths, but more importantly, it gets rid of magic constants in a few different places. --- src/common.cpp | 2 +- src/env_dispatch.cpp | 6 +++--- src/fish_indent.cpp | 2 +- src/fish_key_reader.cpp | 3 ++- src/fish_tests.cpp | 26 +++++++++++++------------- src/future_feature_flags.cpp | 2 +- src/history_file.cpp | 10 +++++----- src/postfork.cpp | 4 ++-- src/signal.cpp | 8 ++++---- 9 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/common.cpp b/src/common.cpp index 98e3852e5..458838181 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1966,7 +1966,7 @@ bool is_console_session() { ASSERT_IS_MAIN_THREAD(); const char *tty_name = ttyname(0); - auto len = strlen("/dev/tty"); + constexpr auto len = const_strlen("/dev/tty"); const char *TERM = getenv("TERM"); return // Test that the tty matches /dev/(console|dcons|tty[uv\d]) diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index 35052206f..376e185be 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -461,9 +461,9 @@ static bool does_term_support_setting_title(const environment_t &vars) { const wcstring term_str = term_var->as_string(); const wchar_t *term = term_str.c_str(); bool recognized = contains(title_terms, term_var->as_string()); - if (!recognized) recognized = !std::wcsncmp(term, L"xterm-", std::wcslen(L"xterm-")); - if (!recognized) recognized = !std::wcsncmp(term, L"screen-", std::wcslen(L"screen-")); - if (!recognized) recognized = !std::wcsncmp(term, L"tmux-", std::wcslen(L"tmux-")); + if (!recognized) recognized = !std::wcsncmp(term, L"xterm-", const_strlen(L"xterm-")); + if (!recognized) recognized = !std::wcsncmp(term, L"screen-", const_strlen(L"screen-")); + if (!recognized) recognized = !std::wcsncmp(term, L"tmux-", const_strlen(L"tmux-")); if (!recognized) { if (std::wcscmp(term, L"linux") == 0) return false; if (std::wcscmp(term, L"dumb") == 0) return false; diff --git a/src/fish_indent.cpp b/src/fish_indent.cpp index e1b75058c..c4a43ab41 100644 --- a/src/fish_indent.cpp +++ b/src/fish_indent.cpp @@ -602,7 +602,7 @@ struct pretty_printer_t { bool added_newline = emit_gap_text_before(node.range, flags); wcstring text = source.substr(node.range.start, node.range.length); if (added_newline && !text.empty() && text.front() == L'\n') { - text = text.substr(strlen("\n")); + text = text.substr(const_strlen("\n")); } emit_gap_text(text, flags); } diff --git a/src/fish_key_reader.cpp b/src/fish_key_reader.cpp index 63cb4fc85..39c7bfe5e 100644 --- a/src/fish_key_reader.cpp +++ b/src/fish_key_reader.cpp @@ -63,7 +63,8 @@ static bool should_exit(wchar_t wc) { std::fwprintf(stderr, L"Press [ctrl-%c] again to exit\n", shell_modes.c_cc[VEOF] + 0x40); return false; } - return std::memcmp(recent_chars, "exit", 4) == 0 || std::memcmp(recent_chars, "quit", 4) == 0; + return std::memcmp(recent_chars, "exit", const_strlen("exit")) == 0 || + std::memcmp(recent_chars, "quit", const_strlen("quit")) == 0; } /// Return the name if the recent sequence of characters matches a known terminfo sequence. diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 01ecc876f..808763283 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -1497,12 +1497,12 @@ static void test_parse_util_cmdsubst_extent() { } parse_util_cmdsubst_extent(a, 8, &begin, &end); - if (begin != a + std::wcslen(L"echo (")) { + if (begin != a + const_strlen(L"echo (")) { err(L"parse_util_cmdsubst_extent failed on line %ld", (long)__LINE__); } parse_util_cmdsubst_extent(a, 17, &begin, &end); - if (begin != a + std::wcslen(L"echo (echo (")) { + if (begin != a + const_strlen(L"echo (echo (")) { err(L"parse_util_cmdsubst_extent failed on line %ld", (long)__LINE__); } } @@ -1811,7 +1811,7 @@ static void test_escape_sequences() { err(L"test_escape_sequences failed on line %d\n", __LINE__); if (lc.escape_code_length(L"\x1B[2J") != 4) err(L"test_escape_sequences failed on line %d\n", __LINE__); - if (lc.escape_code_length(L"\x1B[38;5;123mABC") != std::strlen("\x1B[38;5;123m")) + if (lc.escape_code_length(L"\x1B[38;5;123mABC") != const_strlen("\x1B[38;5;123m")) err(L"test_escape_sequences failed on line %d\n", __LINE__); if (lc.escape_code_length(L"\x1B@") != 2) err(L"test_escape_sequences failed on line %d\n", __LINE__); @@ -2285,41 +2285,41 @@ static void test_abbreviations() { result = expand_abbreviation_in_command(L"gc somebranch", 0, vars); if (!result) err(L"Command not expanded on line %ld", (long)__LINE__); - result = expand_abbreviation_in_command(L"gc somebranch", std::wcslen(L"gc"), vars); + result = expand_abbreviation_in_command(L"gc somebranch", const_strlen(L"gc"), vars); if (!result) err(L"gc not expanded"); if (result != L"git checkout somebranch") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // Space separation. - result = expand_abbreviation_in_command(L"gx somebranch", std::wcslen(L"gc"), vars); + result = expand_abbreviation_in_command(L"gx somebranch", const_strlen(L"gc"), vars); if (!result) err(L"gx not expanded"); if (result != L"git checkout somebranch") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); - result = expand_abbreviation_in_command(L"echo hi ; gc somebranch", std::wcslen(L"echo hi ; g"), - vars); + result = expand_abbreviation_in_command(L"echo hi ; gc somebranch", + const_strlen(L"echo hi ; g"), vars); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"echo hi ; git checkout somebranch") err(L"gc incorrectly expanded on line %ld", (long)__LINE__); result = expand_abbreviation_in_command(L"echo (echo (echo (echo (gc ", - std::wcslen(L"echo (echo (echo (echo (gc"), vars); + const_strlen(L"echo (echo (echo (echo (gc"), vars); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"echo (echo (echo (echo (git checkout ") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // If commands should be expanded. - result = expand_abbreviation_in_command(L"if gc", std::wcslen(L"if gc"), vars); + result = expand_abbreviation_in_command(L"if gc", const_strlen(L"if gc"), vars); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"if git checkout") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // Others should not be. - result = expand_abbreviation_in_command(L"of gc", std::wcslen(L"of gc"), vars); + result = expand_abbreviation_in_command(L"of gc", const_strlen(L"of gc"), vars); if (result) err(L"gc incorrectly expanded on line %ld", (long)__LINE__); // Others should not be. - result = expand_abbreviation_in_command(L"command gc", std::wcslen(L"command gc"), vars); + result = expand_abbreviation_in_command(L"command gc", const_strlen(L"command gc"), vars); if (result) err(L"gc incorrectly expanded on line %ld", (long)__LINE__); vars.pop(); @@ -3824,10 +3824,10 @@ static void test_universal_ok_to_save() { // Ensure we don't try to save after reading from a newer fish. say(L"Testing universal Ok to save"); if (system("mkdir -p test/fish_uvars_test/")) err(L"mkdir failed"); - const char *contents = "# VERSION: 99999.99\n"; + constexpr const char *contents = "# VERSION: 99999.99\n"; FILE *fp = fopen(wcs2string(UVARS_TEST_PATH).c_str(), "w"); assert(fp && "Failed to open UVARS_TEST_PATH for writing"); - fwrite(contents, std::strlen(contents), 1, fp); + fwrite(contents, const_strlen(contents), 1, fp); fclose(fp); file_id_t before_id = file_id_for_path(UVARS_TEST_PATH); diff --git a/src/future_feature_flags.cpp b/src/future_feature_flags.cpp index a2305bbc7..54673d3b1 100644 --- a/src/future_feature_flags.cpp +++ b/src/future_feature_flags.cpp @@ -40,7 +40,7 @@ void features_t::set_from_string(const wcstring &str) { // A "no-" prefix inverts the sense. if (string_prefixes_string(L"no-", name)) { value = false; - name += 3; // std::wcslen(L"no-") + name += const_strlen("no-"); } // Look for a feature with this name. If we don't find it, assume it's a group name and set // all features whose group contain it. Do nothing even if the string is unrecognized; this diff --git a/src/history_file.cpp b/src/history_file.cpp index 909e670f9..7fb0fdc86 100644 --- a/src/history_file.cpp +++ b/src/history_file.cpp @@ -370,18 +370,18 @@ static size_t offset_of_next_item_fish_2_0(const history_file_contents_t &conten // Hackish: fish 1.x rewriting a fish 2.0 history file can produce lines with lots of // leading "- cmd: - cmd: - cmd:". Trim all but one leading "- cmd:". - const char *double_cmd = "- cmd: - cmd: "; - const size_t double_cmd_len = std::strlen(double_cmd); + constexpr const char *double_cmd = "- cmd: - cmd: "; + constexpr const size_t double_cmd_len = const_strlen(double_cmd); while (static_cast(a_newline - line_start) > double_cmd_len && !std::memcmp(line_start, double_cmd, double_cmd_len)) { // Skip over just one of the - cmd. In the end there will be just one left. - line_start += std::strlen("- cmd: "); + line_start += const_strlen("- cmd: "); } // Hackish: fish 1.x rewriting a fish 2.0 history file can produce commands like "when: // 123456". Ignore those. - const char *cmd_when = "- cmd: when:"; - const size_t cmd_when_len = std::strlen(cmd_when); + constexpr const char *cmd_when = "- cmd: when:"; + constexpr const size_t cmd_when_len = const_strlen(cmd_when); if (static_cast(a_newline - line_start) >= cmd_when_len && !std::memcmp(line_start, cmd_when, cmd_when_len)) { continue; diff --git a/src/postfork.cpp b/src/postfork.cpp index fd1d774d5..0bdebf025 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -425,9 +425,9 @@ static char *get_interpreter(const char *command, char *buffer, size_t buff_size close(fd); } - if (std::strncmp(buffer, "#! /", 4) == 0) { + if (std::strncmp(buffer, "#! /", const_strlen("#! /")) == 0) { return buffer + 3; - } else if (std::strncmp(buffer, "#!/", 3) == 0) { + } else if (std::strncmp(buffer, "#!/", const_strlen("#!/")) == 0) { return buffer + 2; } return nullptr; diff --git a/src/signal.cpp b/src/signal.cpp index 0b01112b9..c763b861d 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -145,9 +145,9 @@ static const struct lookup_entry signal_table[] = { /// Test if \c name is a string describing the signal named \c canonical. static int match_signal_name(const wchar_t *canonical, const wchar_t *name) { - if (wcsncasecmp(name, L"sig", 3) == 0) name += 3; + if (wcsncasecmp(name, L"sig", const_strlen("sig")) == 0) name += 3; - return wcscasecmp(canonical + 3, name) == 0; + return wcscasecmp(canonical + const_strlen("sig"), name) == 0; } int wcs2sig(const wchar_t *str) { @@ -187,10 +187,10 @@ static const pid_t s_main_pid = getpid(); /// It's possible that we receive a signal after we have forked, but before we have reset the signal /// handlers (or even run the pthread_atfork calls). In that event we will do something dumb like -/// swallow SIGINT. Ensure that doesn't happen. Check if we are the main fish process; if not reset +/// swallow SIGINT. Ensure that doesn't happen. Check if we are the main fish process; if not, reset /// and re-raise the signal. \return whether we re-raised the signal. static bool reraise_if_forked_child(int sig) { - // Don't use is_forked_child, that relies on atfork handlers which maybe have not run yet. + // Don't use is_forked_child: it relies on atfork handlers which may have not yet run. if (getpid() == s_main_pid) { return false; }