C++ environment variable expansion

Question

I wrote the following a while ago (using regex code I found on Stack Overflow), and I have the following concern I'd like addressed: should there be an std::move() around the passing of the string into the parameter in the second version of the function, ExpandEnv(std::string const &text), to the rvalue version? Conversely, is there any reason not to use it?

inline std::string GetEnv(char const var[])
{
#if defined _WIN32 // getenv() is deprecated on Windows
    char *buf{nullptr};
    size_t sz;
    std::string val;
    if (_dupenv_s(&buf, &sz, var) || buf == nullptr) return val;
    val = buf;
    free(buf);
    return val;
#else
    return getenv(var);
#endif
}

inline std::string ExpandEnv(std::string &&text)
{
    static const std::regex ENV{"\\$\\{([^}]+)\\}"};
    std::smatch match;
    while (std::regex_search(text, match, ENV)) text.replace(match.begin()->first, match[0].second, GetEnv(match[1].str().c_str()));
    return text;
}

inline std::string ExpandEnv(std::string const &text)
{
    return ExpandEnv(std::string(text));
}

Answer 1

You don't need to overload ExpandEnv. In the const-ref overload, your action is to make a copy of the argument and pass that to the rvalue-ref version.

There's a simple way to get the compiler to do that, and it's pass by value. Often overlooked, this is exactly the right way to get a modifiable value in your function, by move if possible, and by value if not:

inline std::string ExpandEnv(std::string text)
{
    //...implementation
}

To answer your specific question (even though we've eliminated the relevant code), there's no need for std::move(), as std::string(...) is already an rvalue expression.

---

Other notes:

• Your local style guide may say otherwise, but we normally use PascalCase for names of composite types rather than for functions. Consistency with the rest of the code base and any libraries may be a factor here, though.

• That Windows code instead of getenv() is horrible - I hope I never have to target Windows! Can _dupenv_s() return true when it has allocated buf? If so, the error path also needs to free(buf).

• buf == nullptr can be written simply as !buf.

• In the Windows path, you can avoid the need for local val by giving buf to a unique_ptr to look after it for you (so you can simply return buf; or return {}; for the success and failure cases, respectively):

    char *buf{nullptr};
    size_t sz;
    auto failed = _dupenv_s(&buf, &sz, var);
    std::unique_ptr<char,malloc_deleter> m{buf}; // ensure buf is released automatically
    return failed ? std::string{} : std::string{buf, sz};
  

• Actually, now I've read up on the Windows function, there seems to be no reason not to use std::getenv() - the only difference in behaviour seems to be if a null pointer is passed as argument, and we never do that.

• You might want to use a raw string literal to reduce those doubled-backslashes: \$\{([^}]+)\} may be slightly easier to read.

• Really, really split that single-line while into separate lines for the condition and statement. It may even be worth making it a block, and giving names to the match and replacement - notably for match[0] which you also write as match.begin() in that expression.

• I'd probably write the argument to GetEnv() as char const* rather than char const[], to help distinguish from actual array arguments (in templates, where the size is a template argument).

• Do you want to provide a means to escape ${ when it appears literally in the input?

---

#Reworked code

#include <string>
#include <regex>
#include <cstdlib>

inline std::string expand_env(std::string text)
{
    static const std::regex env_re{R"--(\$\{([^}]+)\})--"};
    std::smatch match;
    while (std::regex_search(text, match, env_re)) {
        auto const from = match[0];
        auto const var_name = match[1].str().c_str();
        text.replace(from.first, from.second, std::getenv(var_name));
    }
    return text;
}

#include <iostream>
int main()
{
    std::cout << expand_env("${HOME} is where the heart is.\n");
}