base: Return bool instead of void in EnvVarGetter::SetEnv.

Returning bool instead of void in this function is better, because with that
we can know if we had succeeded when tried to set an environment variable or not.
Also it makes this function more testable.

BUG=None
TEST=out/Debug/base_unittests --gtest_filter=EnvVarTest.*

Signed-off-by: Thiago Farina <tfarina@chromium.org>

Review URL: http://codereview.chromium.org/2891024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52602 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 12 insertions, 9 deletions

diff --git a/base/env_var.cc b/base/env_var.cc
index bcdadb9..086191e 100644
--- a/base/env_var.cc
+++ b/base/env_var.cc
@@ -40,8 +40,8 @@ class EnvVarGetterImpl : public base::EnvVarGetter {
     return GetEnvImpl(alternate_case_var.c_str(), result);
   }
 
-  virtual void SetEnv(const char* variable_name, const std::string& new_value) {
-    SetEnvImpl(variable_name, new_value);
+  virtual bool SetEnv(const char* variable_name, const std::string& new_value) {
+    return SetEnvImpl(variable_name, new_value);
   }
 
  private:
@@ -71,12 +71,14 @@ class EnvVarGetterImpl : public base::EnvVarGetter {
 #endif
   }
 
-  void SetEnvImpl(const char* variable_name, const std::string& new_value) {
+  bool SetEnvImpl(const char* variable_name, const std::string& new_value) {
 #if defined(OS_POSIX)
-    setenv(variable_name, new_value.c_str(), 1);
+    // On success, zero is returned.
+    return setenv(variable_name, new_value.c_str(), 1) == 0;
 #elif defined(OS_WIN)
-    ::SetEnvironmentVariable(ASCIIToWide(variable_name).c_str(),
-                             ASCIIToWide(new_value).c_str());
+    // On success, a nonzero is returned.
+    return ::SetEnvironmentVariable(ASCIIToWide(variable_name).c_str(),
+                                    ASCIIToWide(new_value).c_str()) != 0;
 #endif
   }
 };
diff --git a/base/env_var.h b/base/env_var.h
index 3cc2399..f0fe81f 100644
--- a/base/env_var.h
+++ b/base/env_var.h
@@ -23,7 +23,8 @@ class EnvVarGetter {
   // Syntactic sugar for GetEnv(variable_name, NULL);
   virtual bool HasEnv(const char* variable_name);
 
-  virtual void SetEnv(const char* variable_name,
+  // Returns true on success, otherwise returns false.
+  virtual bool SetEnv(const char* variable_name,
                       const std::string& new_value) = 0;
 
   // Create an instance of EnvVarGetter
diff --git a/base/env_var_unittest.cc b/base/env_var_unittest.cc
index 5587878..d80d997 100644
--- a/base/env_var_unittest.cc
+++ b/base/env_var_unittest.cc
@@ -27,7 +27,7 @@ TEST_F(EnvVarTest, SetEnvVar) {
   const char kFooUpper[] = "FOO";
   const char kFooLower[] = "foo";
   scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
-  env->SetEnv(kFooUpper, kFooLower);
+  EXPECT_TRUE(env->SetEnv(kFooUpper, kFooLower));
 
   // Now verify that the environment has the new variable.
   EXPECT_TRUE(env->HasEnv(kFooUpper));
diff --git a/base/xdg_util_unittest.cc b/base/xdg_util_unittest.cc
index 813242b..b821921 100644
--- a/base/xdg_util_unittest.cc
+++ b/base/xdg_util_unittest.cc
@@ -19,7 +19,7 @@ namespace {
 class MockEnvVarGetter : public base::EnvVarGetter {
  public:
   MOCK_METHOD2(GetEnv, bool(const char*, std::string* result));
-  MOCK_METHOD2(SetEnv, void(const char*, const std::string& new_value));
+  MOCK_METHOD2(SetEnv, bool(const char*, const std::string& new_value));
 };
 
 const char* kGnome = "gnome";