operations_test.cpp does not correctly use setenv

[Daniel Krügler <daniel.kruegler@…>]

The constructor of guarded_env_var::previous_value never sets the member variable m_name but does instead set the member m_string with the environment name. This means that the destructor of previous_value always calls unsetenv or setenv with an empty name string.

The required fix seems to be to replace in line 1711

      : m_string(name)

by

      : m_name(name)

As I side note I would like to remark that the return values of the Windows emulations between line 62 and 75

inline int setenv(const char* name, const fs::path::value_type* val, int) 
{
  return SetEnvironmentVariableW(convert(name).c_str(), val); 
}

inline int setenv(const char* name, const char* val, int) 
{
  return SetEnvironmentVariableW(convert(name).c_str(), convert(val).c_str()); 
}

inline int unsetenv(const char* name) 
{ 
  return SetEnvironmentVariableW(convert(name).c_str(), 0); 
}

are inverted versus the POSIX functions, ::SetEnvironmentVariableW returns 0 on failure and non-zero on success. While a proper fix is easy to realize along the lines of

inline int setenv(const char* name, const fs::path::value_type* val, int) 
{
  return SetEnvironmentVariableW(convert(name).c_str(), val) ? 0 : -1; 
}

inline int setenv(const char* name, const char* val, int) 
{
  return SetEnvironmentVariableW(convert(name).c_str(), convert(val).c_str()) ? 0 : -1; 
}

inline int unsetenv(const char* name) 
{ 
  return SetEnvironmentVariableW(convert(name).c_str(), 0) ? 0 : -1; 
}

a much simpler fix would be to define them as void functions because the return values are never tested.

[Daniel Krügler <daniel.kruegler@…>]

While I still think that the suggested changes are formally correct, I would like to mention that the usage of SetEnvironmentVariableW as a means to change the environment variables of the current process can be problematic for some runtimes, in particular for the MSV CRT. The problem here is, that albeit these functions properly update the environment variable values of the OS, they *won't* update the copies of these values kept by the CRT like _environ or any function that depends on these, e.g. std::getenv. Some details of this problem are explained here:

​http://www.codeproject.com/Articles/43029/When-what-you-set-is-not-what-you-get-SetEnvironme

We recently stumbled across this very same problem when we used a similar approach to change the environment variables of the process by means of SetEnvironmentVariableW and wondered why our CORBA::ORB_init from OmniOrb didn't react as we expected: It queries getenv and the environment hasn't been changed for this function. The proper solution for MSVCRT-based systems is to use _putenv instead - it can be considered as the Microsoft pendant of the POSIX setenv function (It does *not* have the problematic semantics of the POSIX putenv function).

[Beman Dawes]

Applied both the name fix and the suggested setenv* void fix. Added comment explaining choice of void fix rather than _putenv fix. Choose the void fix because the Windows Runtime does not support _putenv - only the MSVCRT does that - and we need to support the Windows Runtime.

Thanks,

--Beman