[sandbox] Modernize Environment::GetVar to use std::optional

Updates GetVar usage in //sandbox to use the modern std::optional API
instead of out-parameters.

Bug: 400758498

Change-Id: I75a2dd58499e41925eb433b58e0c71e3984f195f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6425903
Commit-Queue: Helmut Januschka <helmut@januschka.com>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442428}

sandbox/linux/suid/client/setuid_sandbox_client.cc

@@ -9,6 +9,7 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -31,10 +32,11 @@ bool IsFileSystemAccessDenied() {
 }
 
 int GetHelperApi(base::Environment* env) {
-  std::string api_string;
+  std::optional<std::string> api_string =
+      env->GetVar(sandbox::kSandboxEnvironmentApiProvides);
   int api_number = 0;  // Assume API version 0 if no environment was found.
-  if (env->GetVar(sandbox::kSandboxEnvironmentApiProvides, &api_string) &&
+  if (api_string.has_value() &&
-      !base::StringToInt(api_string, &api_number)) {
+      !base::StringToInt(api_string.value(), &api_number)) {
     // It's an error if we could not convert the API number.
     api_number = -1;
   }
@@ -44,10 +46,9 @@ int GetHelperApi(base::Environment* env) {
 // Convert |var_name| from the environment |env| to an int.
 // Return -1 if the variable does not exist or the value cannot be converted.
 int EnvToInt(base::Environment* env, const char* var_name) {
-  std::string var_string;
+  std::string var_string = env->GetVar(var_name).value_or(std::string());
   int var_value = -1;
-  if (env->GetVar(var_name, &var_string) &&
+  if (!var_string.empty() && !base::StringToInt(var_string, &var_value)) {
-      !base::StringToInt(var_string, &var_value)) {
     var_value = -1;
   }
   return var_value;

sandbox/linux/suid/client/setuid_sandbox_host.cc

@@ -16,6 +16,7 @@
 #include <unistd.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -89,11 +90,12 @@ void SaveSUIDUnsafeEnvironmentVariables(base::Environment* env) {
     if (!saved_env_var)
       continue;
 
-    std::string value;
+    std::optional<std::string> value = env->GetVar(env_var);
-    if (env->GetVar(env_var, &value))
+    if (value.has_value()) {
-      env->SetVar(*saved_env_var, value);
+      env->SetVar(*saved_env_var, *value);
-    else
+    } else {
       env->UnSetVar(*saved_env_var);
+    }
   }
 }
 

sandbox/linux/suid/client/setuid_sandbox_host_unittest.cc

@@ -5,6 +5,7 @@
 #include "sandbox/linux/suid/client/setuid_sandbox_host.h"
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <tuple>
 
@@ -20,11 +21,8 @@ TEST(SetuidSandboxHost, SetupLaunchEnvironment) {
   std::unique_ptr<base::Environment> env(base::Environment::Create());
   EXPECT_TRUE(env != NULL);
 
-  std::string saved_ld_preload;
-  bool environment_had_ld_preload;
-  // First, back-up the real LD_PRELOAD if any.
-  environment_had_ld_preload = env->GetVar("LD_PRELOAD", &saved_ld_preload);
   // Setup environment variables to save or not save.
+  std::optional<std::string> saved_ld_preload = env->GetVar("LD_PRELOAD");
   EXPECT_TRUE(env->SetVar("LD_PRELOAD", kTestValue));
   EXPECT_TRUE(env->UnSetVar("LD_ORIGIN_PATH"));
 
@@ -38,24 +36,26 @@ TEST(SetuidSandboxHost, SetupLaunchEnvironment) {
   sandbox_host->SetupLaunchEnvironment();
 
   // Check if the requested API environment was set.
-  std::string api_request;
+  std::optional<std::string> api_request =
-  EXPECT_TRUE(env->GetVar(kSandboxEnvironmentApiRequest, &api_request));
+      env->GetVar(kSandboxEnvironmentApiRequest);
+  EXPECT_TRUE(api_request.has_value());
   int api_request_num;
-  EXPECT_TRUE(base::StringToInt(api_request, &api_request_num));
+  EXPECT_TRUE(base::StringToInt(*api_request, &api_request_num));
   EXPECT_EQ(api_request_num, kSUIDSandboxApiNumber);
 
   // Now check if LD_PRELOAD was saved to SANDBOX_LD_PRELOAD.
-  std::string sandbox_ld_preload;
+  std::optional<std::string> sandbox_ld_preload =
-  EXPECT_TRUE(env->GetVar("SANDBOX_LD_PRELOAD", &sandbox_ld_preload));
+      env->GetVar("SANDBOX_LD_PRELOAD");
-  EXPECT_EQ(sandbox_ld_preload, kTestValue);
+  EXPECT_TRUE(sandbox_ld_preload.has_value());
+  EXPECT_EQ(*sandbox_ld_preload, kTestValue);
 
   // Check that LD_ORIGIN_PATH was not saved.
   EXPECT_FALSE(env->HasVar("SANDBOX_LD_ORIGIN_PATH"));
 
   // We should not forget to restore LD_PRELOAD at the end, or this environment
   // variable will affect the next running tests!
-  if (environment_had_ld_preload) {
+  if (saved_ld_preload.has_value()) {
-    EXPECT_TRUE(env->SetVar("LD_PRELOAD", saved_ld_preload));
+    EXPECT_TRUE(env->SetVar("LD_PRELOAD", *saved_ld_preload));
   } else {
     EXPECT_TRUE(env->UnSetVar("LD_PRELOAD"));
   }

sandbox/mac/seatbelt_unittest.cc

@@ -11,6 +11,7 @@
 #include <unistd.h>
 
 #include <iterator>
+#include <optional>
 
 #include "base/containers/span.h"
 #include "base/files/file.h"
@@ -60,11 +61,11 @@ MULTIPROCESS_TEST_MAIN(Ftruncate) {
 
   std::unique_ptr<base::Environment> env = base::Environment::Create();
 
-  std::string fd_string;
+  std::optional<std::string> fd_string = env->GetVar("FD_TO_TRUNCATE");
-  CHECK(env->GetVar("FD_TO_TRUNCATE", &fd_string));
+  CHECK(fd_string.has_value());
 
   int fd;
-  CHECK(base::StringToInt(fd_string, &fd));
+  CHECK(base::StringToInt(*fd_string, &fd));
 
   const char kTestBuf[] = "hello";
   CHECK_EQ(static_cast<ssize_t>(strlen(kTestBuf)),

sandbox/win/src/address_sanitizer_test.cc

@@ -10,6 +10,7 @@
 #include <stdio.h>
 
 #include <memory>
+#include <optional>
 
 #include "base/environment.h"
 #include "base/files/file_path.h"
@@ -27,20 +28,20 @@ class AddressSanitizerTests : public ::testing::Test {
  public:
   void SetUp() override {
     env_ = base::Environment::Create();
-    had_asan_options_ = env_->GetVar("ASAN_OPTIONS", &old_asan_options_);
+    old_asan_options_ = env_->GetVar("ASAN_OPTIONS");
   }
 
   void TearDown() override {
-    if (had_asan_options_)
+    if (old_asan_options_.has_value()) {
-      ASSERT_TRUE(env_->SetVar("ASAN_OPTIONS", old_asan_options_));
+      ASSERT_TRUE(env_->SetVar("ASAN_OPTIONS", *old_asan_options_));
-    else
+    } else {
       env_->UnSetVar("ASAN_OPTIONS");
+    }
   }
 
  protected:
   std::unique_ptr<base::Environment> env_;
-  bool had_asan_options_;
+  std::optional<std::string> old_asan_options_;
-  std::string old_asan_options_;
 };
 
 SBOX_TESTS_COMMAND int AddressSanitizerTests_Report(int argc, wchar_t** argv) {