From: Joel Rosdahl Date: Mon, 27 Jul 2020 13:25:04 +0000 (+0200) Subject: C++-ify subst_env_in_string X-Git-Tag: v4.0~284 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ab3c68b266d0d2d24c46dd1ef32e74e35ec388d6;p=thirdparty%2Fccache.git C++-ify subst_env_in_string --- diff --git a/src/Config.cpp b/src/Config.cpp index 02c5c5add..f58ecc640 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -196,21 +196,6 @@ format_bool(bool value) return value ? "true" : "false"; } -std::string -parse_env_string(const std::string& value) -{ - char* errmsg = nullptr; - char* substituted = subst_env_in_string(value.c_str(), &errmsg); - if (!substituted) { - std::string error_message = errmsg; - free(errmsg); - throw Error(error_message); - } - std::string result = substituted; - free(substituted); - return result; -} - double parse_double(const std::string& value) { @@ -673,7 +658,7 @@ Config::set_item(const std::string& key, switch (it->second) { case ConfigItem::base_dir: - m_base_dir = parse_env_string(value); + m_base_dir = Util::expand_environment_variables(value); if (!m_base_dir.empty()) { // The empty string means "disable" verify_absolute_path(m_base_dir); m_base_dir = Util::normalize_absolute_path(m_base_dir); @@ -681,7 +666,7 @@ Config::set_item(const std::string& key, break; case ConfigItem::cache_dir: - set_cache_dir(parse_env_string(value)); + set_cache_dir(Util::expand_environment_variables(value)); break; case ConfigItem::cache_dir_levels: @@ -733,7 +718,7 @@ Config::set_item(const std::string& key, break; case ConfigItem::extra_files_to_hash: - m_extra_files_to_hash = parse_env_string(value); + m_extra_files_to_hash = Util::expand_environment_variables(value); break; case ConfigItem::file_clone: @@ -749,11 +734,11 @@ Config::set_item(const std::string& key, break; case ConfigItem::ignore_headers_in_manifest: - m_ignore_headers_in_manifest = parse_env_string(value); + m_ignore_headers_in_manifest = Util::expand_environment_variables(value); break; case ConfigItem::ignore_options: - m_ignore_options = parse_env_string(value); + m_ignore_options = Util::expand_environment_variables(value); break; case ConfigItem::inode_cache: @@ -769,7 +754,7 @@ Config::set_item(const std::string& key, break; case ConfigItem::log_file: - m_log_file = parse_env_string(value); + m_log_file = Util::expand_environment_variables(value); break; case ConfigItem::max_files: @@ -781,7 +766,7 @@ Config::set_item(const std::string& key, break; case ConfigItem::path: - m_path = parse_env_string(value); + m_path = Util::expand_environment_variables(value); break; case ConfigItem::pch_external_checksum: @@ -789,11 +774,11 @@ Config::set_item(const std::string& key, break; case ConfigItem::prefix_command: - m_prefix_command = parse_env_string(value); + m_prefix_command = Util::expand_environment_variables(value); break; case ConfigItem::prefix_command_cpp: - m_prefix_command_cpp = parse_env_string(value); + m_prefix_command_cpp = Util::expand_environment_variables(value); break; case ConfigItem::read_only: @@ -821,7 +806,7 @@ Config::set_item(const std::string& key, break; case ConfigItem::temporary_dir: - m_temporary_dir = parse_env_string(value); + m_temporary_dir = Util::expand_environment_variables(value); m_temporary_dir_configured_explicitly = true; break; diff --git a/src/Util.cpp b/src/Util.cpp index 0cda41f13..b8b92f89a 100644 --- a/src/Util.cpp +++ b/src/Util.cpp @@ -251,6 +251,49 @@ ends_with(string_view string, string_view suffix) return string.ends_with(suffix); } +std::string +expand_environment_variables(const std::string& str) +{ + std::string result; + const char* left = str.c_str(); + for (const char* right = left; *right; ++right) { + if (*right == '$') { + result.append(left, right - left); + + left = right + 1; + bool curly = *left == '{'; + if (curly) { + ++left; + } + right = left; + while (isalnum(*right) || *right == '_') { + ++right; + } + if (curly && *right != '}') { + throw Error( + fmt::format("syntax error: missing '}}' after \"{}\"", left)); + } + if (right == left) { + // Special case: don't consider a single $ the left of a variable. + result += '$'; + } else { + std::string name(left, right - left); + const char* value = getenv(name.c_str()); + if (!value) { + throw Error(fmt::format("environment variable \"{}\" not set", name)); + } + result += value; + if (!curly) { + --right; + } + left = right + 1; + } + } + } + result += left; + return result; +} + int fallocate(int fd, long new_size) { diff --git a/src/Util.hpp b/src/Util.hpp index f4844bee6..b2cd2844d 100644 --- a/src/Util.hpp +++ b/src/Util.hpp @@ -107,6 +107,11 @@ nonstd::string_view dir_name(nonstd::string_view path); // Return true if suffix is a suffix of string. bool ends_with(nonstd::string_view string, nonstd::string_view suffix); +// Expand all instances of $VAR or ${VAR}, where VAR is an environment variable, +// in `str`. Throws `Error` if one of the environment variables. +[[gnu::warn_unused_result]] std::string +expand_environment_variables(const std::string& str); + // Extends file size to at least new_size by calling posix_fallocate() if // supported, otherwise by writing zeros last to the file. // diff --git a/src/legacy_util.cpp b/src/legacy_util.cpp index 2dafad98c..00c97e302 100644 --- a/src/legacy_util.cpp +++ b/src/legacy_util.cpp @@ -508,80 +508,6 @@ x_rename(const char* oldpath, const char* newpath) #endif } -static bool -expand_variable(const char** str, char** result, char** errmsg) -{ - assert(**str == '$'); - - bool curly; - const char* p = *str + 1; - if (*p == '{') { - curly = true; - ++p; - } else { - curly = false; - } - - const char* q = p; - while (isalnum(*q) || *q == '_') { - ++q; - } - if (curly) { - if (*q != '}') { - *errmsg = format("syntax error: missing '}' after \"%s\"", p); - return false; - } - } - - if (q == p) { - // Special case: don't consider a single $ the start of a variable. - reformat(result, "%s$", *result); - return true; - } - - char* name = x_strndup(p, q - p); - const char* value = getenv(name); - if (!value) { - *errmsg = format("environment variable \"%s\" not set", name); - free(name); - return false; - } - reformat(result, "%s%s", *result, value); - if (!curly) { - --q; - } - *str = q; - free(name); - return true; -} - -// Substitute all instances of $VAR or ${VAR}, where VAR is an environment -// variable, in a string. Caller frees. If one of the environment variables -// doesn't exist, NULL will be returned and *errmsg will be an appropriate -// error message (caller frees). -char* -subst_env_in_string(const char* str, char** errmsg) -{ - assert(errmsg); - *errmsg = nullptr; - - char* result = x_strdup(""); - const char* p = str; // Interval start. - const char* q = str; // Interval end. - for (; *q; ++q) { - if (*q == '$') { - reformat(&result, "%s%.*s", result, (int)(q - p), p); - if (!expand_variable(&q, &result, errmsg)) { - free(result); - return nullptr; - } - p = q + 1; - } - } - reformat(&result, "%s%.*s", result, (int)(q - p), p); - return result; -} - void set_cloexec_flag(int fd) { diff --git a/src/legacy_util.hpp b/src/legacy_util.hpp index ded137fb9..16049292d 100644 --- a/src/legacy_util.hpp +++ b/src/legacy_util.hpp @@ -46,6 +46,5 @@ bool is_full_path(const char* path); void update_mtime(const char* path); void x_exit(int status) ATTR_NORETURN; int x_rename(const char* oldpath, const char* newpath); -char* subst_env_in_string(const char* str, char** errmsg); void set_cloexec_flag(int fd); double time_seconds(); diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp index 7a864a134..844cd6253 100644 --- a/unittest/test_Util.cpp +++ b/unittest/test_Util.cpp @@ -159,6 +159,27 @@ TEST_CASE("Util::ends_with") CHECK_FALSE(Util::ends_with("x", "xy")); } +TEST_CASE("Util::expand_environment_variables") +{ + x_setenv("FOO", "bar"); + + CHECK(Util::expand_environment_variables("") == ""); + CHECK(Util::expand_environment_variables("$FOO") == "bar"); + CHECK(Util::expand_environment_variables("$") == "$"); + CHECK(Util::expand_environment_variables("$FOO $FOO:$FOO") == "bar bar:bar"); + CHECK(Util::expand_environment_variables("x$FOO") == "xbar"); + CHECK(Util::expand_environment_variables("${FOO}x") == "barx"); + + DOCTEST_GCC_SUPPRESS_WARNING_PUSH + DOCTEST_GCC_SUPPRESS_WARNING("-Wunused-result") + CHECK_THROWS_WITH( + (void)Util::expand_environment_variables("$surelydoesntexist"), + "environment variable \"surelydoesntexist\" not set"); + CHECK_THROWS_WITH((void)Util::expand_environment_variables("${FOO"), + "syntax error: missing '}' after \"FOO\""); + DOCTEST_GCC_SUPPRESS_WARNING_POP +} + TEST_CASE("Util::fallocate") { TestContext test_context; diff --git a/unittest/test_legacy_util.cpp b/unittest/test_legacy_util.cpp index c03d6d8d3..0630cbfaf 100644 --- a/unittest/test_legacy_util.cpp +++ b/unittest/test_legacy_util.cpp @@ -28,36 +28,6 @@ TEST_SUITE_BEGIN("legacy_util"); -TEST_CASE("subst_env_in_string") -{ - char* errmsg; - - x_setenv("FOO", "bar"); - - CHECK_STR_EQ_FREE2("bar", subst_env_in_string("$FOO", &errmsg)); - CHECK(!errmsg); - - CHECK_STR_EQ_FREE2("$", subst_env_in_string("$", &errmsg)); - CHECK(!errmsg); - - CHECK_STR_EQ_FREE2("bar bar:bar", - subst_env_in_string("$FOO $FOO:$FOO", &errmsg)); - CHECK(!errmsg); - - CHECK_STR_EQ_FREE2("xbar", subst_env_in_string("x$FOO", &errmsg)); - CHECK(!errmsg); - - CHECK_STR_EQ_FREE2("barx", subst_env_in_string("${FOO}x", &errmsg)); - CHECK(!errmsg); - - CHECK(!subst_env_in_string("$surelydoesntexist", &errmsg)); - CHECK_STR_EQ_FREE2("environment variable \"surelydoesntexist\" not set", - errmsg); - - CHECK(!subst_env_in_string("${FOO", &errmsg)); - CHECK_STR_EQ_FREE2("syntax error: missing '}' after \"FOO\"", errmsg); -} - TEST_CASE("parse_size_with_suffix") { uint64_t size;