Cleanup calls to sapi::StatusOr<>::ValueOrDie()

PiperOrigin-RevId: 304398197
Change-Id: I85d09457a5e27f65c0792fe93aebbd8219801ef6

sandboxed_api/client.cc

@@ -122,7 +122,7 @@ class FunctionCallPreparer {
       // There is no way to figure out whether the protobuf structure has
       // changed or not, so we always serialize the protobuf again and replace
       // the LenValStruct content.
-      std::vector<uint8_t> serialized = SerializeProto(*proto).ValueOrDie();
+      std::vector<uint8_t> serialized = SerializeProto(*proto).value();
       // Reallocate the LV memory to match its length.
       if (lvs->size != serialized.size()) {
         void* newdata = realloc(lvs->data, serialized.size());

sandboxed_api/examples/zlib/main_zlib.cc

@@ -15,6 +15,8 @@
 #include <linux/audit.h>
 #include <sys/syscall.h>
 
+#include <cstdlib>
+
 #include <glog/logging.h>
 #include "absl/base/macros.h"
 #include "sandboxed_api/util/flag.h"
@@ -40,11 +42,13 @@ int main(int argc, char** argv) {
 
   sapi::Sandbox sandbox(sapi::zlib::zlib_sapi_embed_create());
   sapi::zlib::ZlibApi api(&sandbox);
-  if (!sandbox.Init().ok()) {
+  if (auto status = sandbox.Init(); !status.ok()) {
-    LOG(FATAL) << "Couldn't initialize the Sandboxed API Lib";
+    LOG(FATAL) << "Couldn't initialize Sandboxed API for zlib: "
+               << status.message();
   }
 
-  int ret, flush;
+  sapi::StatusOr<int> ret;
+  int flush;
   unsigned have;
   sapi::v::Struct<sapi::zlib::z_stream> strm;
 
@@ -64,11 +68,10 @@ int main(int argc, char** argv) {
   // Allocate deflate state.
   *strm.mutable_data() = sapi::zlib::z_stream{};
 
-  ret = api.deflateInit_(strm.PtrBoth(), Z_DEFAULT_COMPRESSION,
+  if (ret = api.deflateInit_(strm.PtrBoth(), Z_DEFAULT_COMPRESSION,
-                         version.PtrBefore(), sizeof(sapi::zlib::z_stream))
+                             version.PtrBefore(), sizeof(sapi::zlib::z_stream));
-            .ValueOrDie();
+      *ret != Z_OK) {
-  if (ret != Z_OK) {
+    return *ret;
-    return ret;
   }
 
   LOG(INFO) << "Starting decompression";
@@ -81,7 +84,7 @@ int main(int argc, char** argv) {
     }
     if (ferror(stdin)) {
       LOG(INFO) << "Error reading from stdin";
-      (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie();
+      api.deflateEnd(strm.PtrBoth()).IgnoreError();
       return Z_ERRNO;
     }
     flush = feof(stdin) ? Z_FINISH : Z_NO_FLUSH;
@@ -95,10 +98,8 @@ int main(int argc, char** argv) {
       strm.mutable_data()->next_out =
           reinterpret_cast<unsigned char*>(output.GetRemote());
 
-      ret = api.deflate(strm.PtrBoth(), flush)
+      ret = api.deflate(strm.PtrBoth(), flush);  // no bad return value.
-                .ValueOrDie();  // no bad return value.
+      assert(*ret != Z_STREAM_ERROR);            // state not clobbered.
-
-      assert(ret != Z_STREAM_ERROR);  // state not clobbered.
       have = kChunk - strm.data().avail_out;
 
       if (!sandbox.TransferFromSandboxee(&output).ok()) {
@@ -107,7 +108,7 @@ int main(int argc, char** argv) {
       if (fwrite(output.GetLocal(), 1, have, stdout) != have ||
           ferror(stdout)) {
         // Not really necessary as strm did not change from last transfer.
-        (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie();
+        api.deflateEnd(strm.PtrBoth()).IgnoreError();
         return Z_ERRNO;
       }
     } while (strm.data().avail_out == 0);
@@ -115,10 +116,10 @@ int main(int argc, char** argv) {
 
     // done when last data in file processed.
   } while (flush != Z_FINISH);
-  assert(ret == Z_STREAM_END);  // stream will be complete.
+  assert(*ret == Z_STREAM_END);  // stream will be complete.
 
   // clean up and return.
-  (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie();
+  api.deflateEnd(strm.PtrBoth()).IgnoreError();
 
-  return 0;
+  return EXIT_SUCCESS;
 }

sandboxed_api/rpcchannel.cc

@@ -96,9 +96,7 @@ absl::Status RPCChannel::Reallocate(void* old_addr, size_t size,
         absl::StrCat("Reallocate() failed on the remote side: ",
                      fret_or.status().message()));
   }
-  auto fret = std::move(fret_or).ValueOrDie();
+  *new_addr = reinterpret_cast<void*>(fret_or->int_val);
-
-  *new_addr = reinterpret_cast<void*>(fret.int_val);
   return absl::OkStatus();
 }
 

sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc

@@ -26,7 +26,7 @@ ABSL_FLAG(bool, connect_with_handler, true, "Connect using automatic mode.");
 
 namespace {
 
-sandbox2::NetworkProxyClient* proxy_client;
+static sandbox2::NetworkProxyClient* g_proxy_client;
 
 ssize_t ReadFromFd(int fd, uint8_t* buf, size_t size) {
   ssize_t received = 0;
@@ -72,7 +72,7 @@ sapi::StatusOr<struct sockaddr_in6> CreateAddres(int port) {
 }
 
 absl::Status ConnectManually(int s, const struct sockaddr_in6& saddr) {
-  return proxy_client->Connect(
+  return g_proxy_client->Connect(
       s, reinterpret_cast<const struct sockaddr*>(&saddr), sizeof(saddr));
 }
 
@@ -118,19 +118,19 @@ int main(int argc, char** argv) {
   sandbox2_client.SandboxMeHere();
 
   if (absl::GetFlag(FLAGS_connect_with_handler)) {
-    absl::Status status = sandbox2_client.InstallNetworkProxyHandler();
+    if (auto status = sandbox2_client.InstallNetworkProxyHandler();
-    if (!status.ok()) {
+        !status.ok()) {
       LOG(ERROR) << "InstallNetworkProxyHandler() failed: " << status.message();
       return 1;
     }
   } else {
-    proxy_client = sandbox2_client.GetNetworkProxyClient();
+    g_proxy_client = sandbox2_client.GetNetworkProxyClient();
   }
 
   // Receive port number of the server
   int port;
   if (!comms.RecvInt32(&port)) {
-    LOG(ERROR) << "sandboxee_comms->RecvUint32(&crc4) failed";
+    LOG(ERROR) << "Failed to receive port number";
     return 2;
   }
 
@@ -139,13 +139,11 @@ int main(int argc, char** argv) {
     LOG(ERROR) << sock_s.status().message();
     return 3;
   }
-  sandbox2::file_util::fileops::FDCloser client{sock_s.ValueOrDie()};
+  sandbox2::file_util::fileops::FDCloser client(sock_s.value());
 
-  absl::Status status = CommunicationTest(client.get());
+  if (auto status = CommunicationTest(client.get()); !status.ok()) {
-  if (!status.ok()) {
+    LOG(ERROR) << status.message();
-    LOG(ERROR) << sock_s.status().message();
     return 4;
   }
-
   return 0;
 }

sandboxed_api/sandbox2/forkserver.cc

@@ -458,13 +458,10 @@ pid_t ForkServer::ServeRequest() {
       // Send sandboxee pid
       absl::Status status = SendPid(fd_closer1.get());
       SAPI_RAW_CHECK(status.ok(), "sending pid: %s", status.message());
+    } else if (auto pid_or = ReceivePid(fd_closer0.get()); !pid_or.ok()) {
+      SAPI_RAW_LOG(ERROR, "receiving pid: %s", pid_or.status().message());
     } else {
-      auto pid_or = ReceivePid(fd_closer0.get());
+      sandboxee_pid = pid_or.value();
-      if (!pid_or.ok()) {
-        SAPI_RAW_LOG(ERROR, "receiving pid: %s", pid_or.status().message());
-      } else {
-        sandboxee_pid = pid_or.ValueOrDie();
-      }
     }
   } else {
     sandboxee_pid = util::ForkWithFlags(clone_flags);
@@ -493,13 +490,12 @@ pid_t ForkServer::ServeRequest() {
     sandboxee_pid = -1;
     // And the actual sandboxee is forked from the init process, so we need to
     // receive the actual PID.
-    auto pid_or = ReceivePid(fd_closer0.get());
+    if (auto pid_or = ReceivePid(fd_closer0.get()); !pid_or.ok()) {
-    if (!pid_or.ok()) {
       SAPI_RAW_LOG(ERROR, "%s", pid_or.status().message());
       kill(init_pid, SIGKILL);
       init_pid = -1;
     } else {
-      sandboxee_pid = pid_or.ValueOrDie();
+      sandboxee_pid = pid_or.value();
     }
   }
 

sandboxed_api/sandbox2/mounts.cc

@@ -124,9 +124,9 @@ absl::Status ValidateInterpreter(absl::string_view interpreter) {
 std::string ResolveLibraryPath(absl::string_view lib_name,
                                const std::vector<std::string>& search_paths) {
   for (const auto& search_path : search_paths) {
-    auto path_or = ExistingPathInsideDir(search_path, lib_name);
+    if (auto path_or = ExistingPathInsideDir(search_path, lib_name);
-    if (path_or.ok()) {
+        path_or.ok()) {
-      return path_or.ValueOrDie();
+      return path_or.value();
     }
   }
   return "";
@@ -290,14 +290,10 @@ void LogContainer(const std::vector<std::string>& container) {
 
 absl::Status Mounts::AddMappingsForBinary(const std::string& path,
                                           absl::string_view ld_library_path) {
-  auto elf_or = ElfFile::ParseFromFile(
+  SAPI_ASSIGN_OR_RETURN(auto elf, ElfFile::ParseFromFile(
-      path, ElfFile::kGetInterpreter | ElfFile::kLoadImportedLibraries);
+                                 path, ElfFile::kGetInterpreter |
-  if (!elf_or.ok()) {
+                                           ElfFile::kLoadImportedLibraries));
-    return absl::FailedPreconditionError(
+  const std::string& interpreter = elf.interpreter();
-        absl::StrCat("Could not parse ELF file: ", elf_or.status().message()));
-  }
-  auto elf = elf_or.ValueOrDie();
-  const std::string interpreter = elf.interpreter();
 
   if (interpreter.empty()) {
     SAPI_RAW_VLOG(1, "The file %s is not a dynamic executable", path);

sandboxed_api/sandbox2/mounts_test.cc

@@ -88,16 +88,16 @@ TEST(MountTreeTest, TestAddTmpFs) {
 TEST(MountTreeTest, TestMultipleInsertionFileSymlink) {
   Mounts mounts;
 
-  auto result_or = CreateNamedTempFileAndClose(
+  SAPI_ASSERT_OK_AND_ASSIGN(std::string path,
-      file::JoinPath(GetTestTempPath(), "testdir_"));
+                       CreateNamedTempFileAndClose(
-  ASSERT_THAT(result_or.status(), IsOk());
+                           file::JoinPath(GetTestTempPath(), "testdir_")));
-  std::string path = result_or.ValueOrDie();
+  SAPI_ASSERT_OK_AND_ASSIGN(std::string symlink_path,
-  result_or = CreateNamedTempFileAndClose(
+                       CreateNamedTempFileAndClose(
-      file::JoinPath(GetTestTempPath(), "testdir_"));
+                           file::JoinPath(GetTestTempPath(), "testdir_")));
-  ASSERT_THAT(result_or.status(), IsOk());
+
-  std::string symlink_path = result_or.ValueOrDie();
   ASSERT_THAT(unlink(symlink_path.c_str()), Eq(0));
   ASSERT_THAT(symlink(path.c_str(), symlink_path.c_str()), Eq(0));
+
   EXPECT_THAT(mounts.AddFileAt(path, "/a"), IsOk());
   EXPECT_THAT(mounts.AddFileAt(path, "/a"), IsOk());
   EXPECT_THAT(mounts.AddFileAt(symlink_path, "/a"), IsOk());
@@ -106,15 +106,15 @@ TEST(MountTreeTest, TestMultipleInsertionFileSymlink) {
 TEST(MountTreeTest, TestMultipleInsertionDirSymlink) {
   Mounts mounts;
 
-  auto result_or = CreateTempDir(file::JoinPath(GetTestTempPath(), "testdir_"));
+  SAPI_ASSERT_OK_AND_ASSIGN(std::string path, CreateTempDir(file::JoinPath(
-  ASSERT_THAT(result_or.status(), IsOk());
+                                             GetTestTempPath(), "testdir_")));
-  std::string path = result_or.ValueOrDie();
+  SAPI_ASSERT_OK_AND_ASSIGN(std::string symlink_path,
-  result_or = CreateNamedTempFileAndClose(
+                       CreateNamedTempFileAndClose(
-      file::JoinPath(GetTestTempPath(), "testdir_"));
+                           file::JoinPath(GetTestTempPath(), "testdir_")));
-  ASSERT_THAT(result_or.status(), IsOk());
+
-  std::string symlink_path = result_or.ValueOrDie();
   ASSERT_THAT(unlink(symlink_path.c_str()), Eq(0));
   ASSERT_THAT(symlink(path.c_str(), symlink_path.c_str()), Eq(0));
+
   EXPECT_THAT(mounts.AddDirectoryAt(path, "/a"), IsOk());
   EXPECT_THAT(mounts.AddDirectoryAt(path, "/a"), IsOk());
   EXPECT_THAT(mounts.AddDirectoryAt(symlink_path, "/a"), IsOk());
@@ -144,7 +144,7 @@ TEST(MountTreeTest, TestEvilNullByte) {
   Mounts mounts;
   // create the filename with a null byte this way as g4 fix forces newlines
   // otherwise.
-  std::string filename{"/a/b"};
+  std::string filename = "/a/b";
   filename[2] = '\0';
 
   EXPECT_THAT(mounts.AddFile(filename),

sandboxed_api/sandbox2/namespace_test.cc

@@ -62,9 +62,8 @@ TEST(NamespaceTest, FileNamespaceWorks) {
 
 TEST(NamespaceTest, ReadOnlyIsRespected) {
   // Mount temporary file as RO and check that it actually is RO.
-  auto [name, fd] =
+  auto [name, fd] = CreateNamedTempFile(GetTestTempPath("temp_file")).value();
-      CreateNamedTempFile(GetTestTempPath("temp_file")).ValueOrDie();
+  file_util::fileops::FDCloser temp_closer(fd);
-  file_util::fileops::FDCloser temp_closer{fd};
 
   const std::string path = GetTestSourcePath("sandbox2/testcases/namespace");
   {

sandboxed_api/sandbox2/network_proxy/server.cc

@@ -104,13 +104,13 @@ void NetworkProxyServer::NotifySuccess() {
 }
 
 void NetworkProxyServer::NotifyViolation(const struct sockaddr* saddr) {
-  sapi::StatusOr<std::string> result = AddrToString(saddr);
+  if (sapi::StatusOr<std::string> result = AddrToString(saddr); result.ok()) {
-  if (result.ok()) {
+    violation_msg_ = std::move(result).value();
-    violation_msg_ = result.ValueOrDie();
   } else {
     violation_msg_ = std::string(result.status().message());
   }
   violation_occurred_.store(true, std::memory_order_release);
   pthread_kill(monitor_thread_id_, SIGCHLD);
 }
+
 }  // namespace sandbox2

sandboxed_api/sandbox2/policybuilder.cc

@@ -723,7 +723,7 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside,
     SetError(fixed_outside_or.status());
     return *this;
   }
-  auto fixed_outside = std::move(fixed_outside_or.ValueOrDie());
+  auto fixed_outside = std::move(fixed_outside_or).value();
 
   if (absl::StartsWith(fixed_outside, "/proc/self")) {
     SetError(absl::InvalidArgumentError(
@@ -733,13 +733,12 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside,
     return *this;
   }
 
-  auto status = mounts_.AddFileAt(fixed_outside, inside, is_ro);
+  if (auto status = mounts_.AddFileAt(fixed_outside, inside, is_ro);
-  if (!status.ok()) {
+      !status.ok()) {
     SetError(
         absl::InternalError(absl::StrCat("Could not add file ", outside, " => ",
                                          inside, ": ", status.message())));
   }
-
   return *this;
 }
 
@@ -752,10 +751,10 @@ PolicyBuilder& PolicyBuilder::AddLibrariesForBinary(
     SetError(fixed_path_or.status());
     return *this;
   }
-  auto fixed_path = std::move(fixed_path_or.ValueOrDie());
+  auto fixed_path = std::move(fixed_path_or).value();
 
-  auto status = mounts_.AddMappingsForBinary(fixed_path, ld_library_path);
+  if (auto status = mounts_.AddMappingsForBinary(fixed_path, ld_library_path);
-  if (!status.ok()) {
+      !status.ok()) {
     SetError(absl::InternalError(absl::StrCat(
         "Could not add libraries for ", fixed_path, ": ", status.message())));
   }
@@ -782,7 +781,7 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside,
     SetError(fixed_outside_or.status());
     return *this;
   }
-  auto fixed_outside = std::move(fixed_outside_or.ValueOrDie());
+  auto fixed_outside = std::move(fixed_outside_or).value();
   if (absl::StartsWith(fixed_outside, "/proc/self")) {
     SetError(absl::InvalidArgumentError(
         absl::StrCat("Cannot add /proc/self mounts, you need to mount the "
@@ -791,25 +790,22 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside,
     return *this;
   }
 
-  auto status = mounts_.AddDirectoryAt(fixed_outside, inside, is_ro);
+  if (auto status = mounts_.AddDirectoryAt(fixed_outside, inside, is_ro);
-  if (!status.ok()) {
+      !status.ok()) {
     SetError(absl::InternalError(absl::StrCat("Could not add directory ",
                                               outside, " => ", inside, ": ",
                                               status.message())));
   }
-
   return *this;
 }
 
 PolicyBuilder& PolicyBuilder::AddTmpfs(absl::string_view inside, size_t sz) {
   EnableNamespaces();
 
-  auto status = mounts_.AddTmpfs(inside, sz);
+  if (auto status = mounts_.AddTmpfs(inside, sz); !status.ok()) {
-  if (!status.ok()) {
     SetError(absl::InternalError(absl::StrCat("Could not mount tmpfs ", inside,
                                               ": ", status.message())));
   }
-
   return *this;
 }
 

sandboxed_api/sandbox2/policybuilder.h

@@ -390,7 +390,7 @@ class PolicyBuilder final {
   // once.
   // This function will abort if an error happened in any off the PolicyBuilder
   // methods.
-  std::unique_ptr<Policy> BuildOrDie() { return TryBuild().ValueOrDie(); }
+  std::unique_ptr<Policy> BuildOrDie() { return TryBuild().value(); }
 
   // Adds a bind-mount for a file from outside the namespace to inside. This
   // will also create parent directories inside the namespace if needed.

sandboxed_api/sandbox2/policybuilder_test.cc

@@ -142,17 +142,15 @@ TEST_F(PolicyBuilderTest, TestValidateAbsolutePath) {
            "/a/b/c/d/",
            "/a/bAAAAAAAAAAAAAAAAAAAAAA/c/d/",
        }) {
-    auto path_or = PolicyBuilderPeer::ValidateAbsolutePath(bad_path);
+    EXPECT_THAT(PolicyBuilderPeer::ValidateAbsolutePath(bad_path),
-    EXPECT_THAT(path_or.status(), StatusIs(absl::StatusCode::kInvalidArgument));
+                StatusIs(absl::StatusCode::kInvalidArgument));
   }
 
   for (auto const& good_path :
        {"/", "/a/b/c/d", "/a/b/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"}) {
-    auto path_or = PolicyBuilderPeer::ValidateAbsolutePath(good_path);
+    SAPI_ASSERT_OK_AND_ASSIGN(std::string path,
-    EXPECT_THAT(path_or, IsOk());
+                         PolicyBuilderPeer::ValidateAbsolutePath(good_path));
-    if (path_or.ok()) {
+    EXPECT_THAT(path, StrEq(good_path));
-      EXPECT_THAT(path_or.ValueOrDie(), StrEq(good_path));
-    }
   }
 }
 

sandboxed_api/sandbox2/sandbox2.cc

@@ -58,7 +58,7 @@ sapi::StatusOr<Result> Sandbox2::AwaitResultWithTimeout(
 }
 
 Result Sandbox2::AwaitResult() {
-  return AwaitResultWithTimeout(absl::InfiniteDuration()).ValueOrDie();
+  return AwaitResultWithTimeout(absl::InfiniteDuration()).value();
 }
 
 bool Sandbox2::RunAsync() {

sandboxed_api/sandbox2/stack_trace.cc

@@ -145,7 +145,7 @@ std::unique_ptr<Policy> StackTracePeer::GetPolicy(pid_t target_pid,
     LOG(ERROR) << "Creating stack unwinder sandbox policy failed";
     return nullptr;
   }
-  std::unique_ptr<Policy> policy = std::move(policy_or).ValueOrDie();
+  std::unique_ptr<Policy> policy = std::move(policy_or).value();
   auto keep_capabilities = absl::make_unique<std::vector<cap_value_t>>();
   keep_capabilities->push_back(CAP_SYS_PTRACE);
   policy->AllowUnsafeKeepCapabilities(std::move(keep_capabilities));

sandboxed_api/sandbox2/stack_trace_test.cc

@@ -39,13 +39,13 @@
 
 ABSL_DECLARE_FLAG(bool, sandbox_libunwind_crash_handler);
 
+namespace sandbox2 {
+namespace {
+
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using ::testing::Not;
 
-namespace sandbox2 {
-namespace {
-
 // Temporarily overrides a flag, restores the original flag value when it goes
 // out of scope.
 template <typename T>
@@ -71,7 +71,7 @@ void SymbolizationWorksCommon(
   std::vector<std::string> args = {path, "1"};
   auto executor = absl::make_unique<Executor>(path, args);
 
-  std::string temp_filename = CreateNamedTempFileAndClose("/tmp/").ValueOrDie();
+  std::string temp_filename = CreateNamedTempFileAndClose("/tmp/").value();
   file_util::fileops::CopyFile("/proc/cpuinfo", temp_filename, 0444);
   struct TempCleanup {
     ~TempCleanup() { remove(capture->c_str()); }

sandboxed_api/sandbox2/syscall_defs.cc

@@ -60,7 +60,7 @@ std::string SyscallTable::Entry::GetArgumentDescription(uint64_t value,
     case kPath:
       if (auto path_or = util::ReadCPathFromPid(pid, value); path_or.ok()) {
         absl::StrAppendFormat(&ret, " ['%s']",
-                              absl::CHexEscape(path_or.ValueOrDie()));
+                              absl::CHexEscape(path_or.value()));
       } else {
         absl::StrAppend(&ret, " [unreadable path]");
       }

sandboxed_api/sandbox2/testcases/buffer.cc

@@ -29,13 +29,12 @@ int main(int argc, char** argv) {
 
   int testno = atoi(argv[1]);  // NOLINT
   switch (testno) {
-    case 1:  // Dup and map to static FD
+    case 1: {  // Dup and map to static FD
-    {
       auto buffer_or = sandbox2::Buffer::CreateFromFd(3);
       if (!buffer_or) {
         return EXIT_FAILURE;
       }
-      auto buffer = std::move(buffer_or).ValueOrDie();
+      auto buffer = std::move(buffer_or).value();
       uint8_t* buf = buffer->data();
       // Test that we can read data from the executor.
       if (buf[0] != 'A') {
@@ -46,8 +45,7 @@ int main(int argc, char** argv) {
       return EXIT_SUCCESS;
     }
 
-    case 2:  // Send and receive FD
+    case 2: {  // Send and receive FD
-    {
       sandbox2::Comms comms(sandbox2::Comms::kSandbox2ClientCommsFD);
       int fd;
       if (!comms.RecvFD(&fd)) {
@@ -57,7 +55,7 @@ int main(int argc, char** argv) {
       if (!buffer_or) {
         return EXIT_FAILURE;
       }
-      auto buffer = std::move(buffer_or).ValueOrDie();
+      auto buffer = std::move(buffer_or).value();
       uint8_t* buf = buffer->data();
       // Test that we can read data from the executor.
       if (buf[0] != 'A') {
@@ -70,6 +68,6 @@ int main(int argc, char** argv) {
 
     default:
       absl::FPrintF(stderr, "Unknown test: %d\n", testno);
-      return EXIT_FAILURE;
   }
+  return EXIT_FAILURE;
 }

sandboxed_api/sandbox2/testcases/symbolize.cc

@@ -40,7 +40,7 @@ void RunWritable() {
 
   std::string tmpname;
   int tmp_fd;
-  std::tie(tmpname, tmp_fd) = sandbox2::CreateNamedTempFile("tmp").ValueOrDie();
+  std::tie(tmpname, tmp_fd) = sandbox2::CreateNamedTempFile("tmp").value();
   SAPI_RAW_PCHECK(fchmod(tmp_fd, S_IRWXU) == 0, "Fchmod on temporary file");
 
   char buf[4096];

sandboxed_api/sandbox2/unwind/unwind.cc

@@ -165,7 +165,7 @@ void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out,
     SAPI_RAW_LOG(ERROR, "Could not parse /proc/%d/maps", pid);
     return;
   }
-  auto maps = std::move(maps_or).ValueOrDie();
+  auto maps = std::move(maps_or).value();
 
   // Get symbols for each file entry in the maps entry.
   // This is not a very efficient way, so we might want to optimize it.
@@ -190,7 +190,7 @@ void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out,
                      elf_or.status().message());
         continue;
       }
-      auto elf = std::move(elf_or).ValueOrDie();
+      auto elf = std::move(elf_or).value();
 
       for (const auto& symbol : elf.symbols()) {
         if (elf.position_independent()) {

sandboxed_api/sandbox2/util/minielf_test.cc

@@ -23,12 +23,6 @@
 #include "sandboxed_api/sandbox2/util/maps_parser.h"
 #include "sandboxed_api/util/status_matchers.h"
 
-using sapi::IsOk;
-using ::testing::Eq;
-using ::testing::IsTrue;
-using ::testing::Not;
-using ::testing::StrEq;
-
 extern "C" void ExportedFunctionName() {
   // Don't do anything - used to generate a symbol.
 }
@@ -36,6 +30,11 @@ extern "C" void ExportedFunctionName() {
 namespace sandbox2 {
 namespace {
 
+using ::testing::Eq;
+using ::testing::IsTrue;
+using ::testing::Not;
+using ::testing::StrEq;
+
 TEST(MinielfTest, Chrome70) {
   SAPI_ASSERT_OK_AND_ASSIGN(
       ElfFile elf,
@@ -58,9 +57,7 @@ TEST(MinielfTest, SymbolResolutionWorks) {
   fread(maps_buffer, 1, sizeof(maps_buffer), f);
   fclose(f);
 
-  auto maps_or = ParseProcMaps(maps_buffer);
+  SAPI_ASSERT_OK_AND_ASSIGN(std::vector<MapsEntry> maps, ParseProcMaps(maps_buffer));
-  ASSERT_THAT(maps_or.status(), IsOk());
-  std::vector<MapsEntry> maps = maps_or.ValueOrDie();
 
   // Find maps entry that covers this entry.
   uint64_t function_address = reinterpret_cast<uint64_t>(ExportedFunctionName);

sandboxed_api/sandbox2/util/temp_file.cc

@@ -48,7 +48,7 @@ sapi::StatusOr<std::string> CreateNamedTempFileAndClose(
   if (result_or.ok()) {
     std::string path;
     int fd;
-    std::tie(path, fd) = result_or.ValueOrDie();
+    std::tie(path, fd) = std::move(result_or).value();
     close(fd);
     return path;
   }

sandboxed_api/sandbox2/util/temp_file_test.cc

@@ -24,40 +24,39 @@
 #include "sandboxed_api/sandbox2/util/path.h"
 #include "sandboxed_api/util/status_matchers.h"
 
-using sapi::IsOk;
-using sapi::StatusIs;
-using testing::Eq;
-using testing::IsTrue;
-using testing::Ne;
-using testing::StartsWith;
-
 namespace sandbox2 {
 namespace {
 
+using ::sapi::IsOk;
+using ::sapi::StatusIs;
+using ::testing::Eq;
+using ::testing::IsTrue;
+using ::testing::Ne;
+using ::testing::StartsWith;
+
 TEST(TempFileTest, CreateTempDirTest) {
   const std::string prefix = GetTestTempPath("MakeTempDirTest_");
-  auto result_or = CreateTempDir(prefix);
+  SAPI_ASSERT_OK_AND_ASSIGN(std::string path, CreateTempDir(prefix));
-  ASSERT_THAT(result_or.status(), IsOk());
+
-  std::string path = result_or.ValueOrDie();
   EXPECT_THAT(path, StartsWith(prefix));
   EXPECT_THAT(file_util::fileops::Exists(path, false), IsTrue());
-  result_or = CreateTempDir("non_existing_dir/prefix");
+  EXPECT_THAT(CreateTempDir("non_existing_dir/prefix"),
-  EXPECT_THAT(result_or, StatusIs(absl::StatusCode::kUnknown));
+              StatusIs(absl::StatusCode::kUnknown));
 }
 
 TEST(TempFileTest, MakeTempFileTest) {
   const std::string prefix = GetTestTempPath("MakeTempDirTest_");
+
   auto result_or = CreateNamedTempFile(prefix);
   ASSERT_THAT(result_or.status(), IsOk());
-  std::string path;
+  auto [path, fd] = std::move(result_or).value();
-  int fd;
+
-  std::tie(path, fd) = result_or.ValueOrDie();
   EXPECT_THAT(path, StartsWith(prefix));
   EXPECT_THAT(file_util::fileops::Exists(path, false), IsTrue());
   EXPECT_THAT(fcntl(fd, F_GETFD), Ne(-1));
   EXPECT_THAT(close(fd), Eq(0));
-  result_or = CreateNamedTempFile("non_existing_dir/prefix");
+  EXPECT_THAT(CreateNamedTempFile("non_existing_dir/prefix"),
-  EXPECT_THAT(result_or, StatusIs(absl::StatusCode::kUnknown));
+              StatusIs(absl::StatusCode::kUnknown));
 }
 
 }  // namespace

sandboxed_api/sapi_test.cc

@@ -200,59 +200,51 @@ TEST(SandboxTest, RestartTransactionSandboxFD) {
 
 // Make sure we can recover from a dying sandbox.
 TEST(SandboxTest, RestartSandboxAfterCrash) {
-  sapi::BasicTransaction st{absl::make_unique<SumSandbox>()};
+  SumSandbox sandbox;
+  ASSERT_THAT(sandbox.Init(), IsOk());
+  SumApi api(&sandbox);
 
-  auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status {
+  // Crash the sandbox.
-    SumApi sumapi(sandbox);
+  EXPECT_THAT(api.crash(), StatusIs(absl::StatusCode::kUnavailable));
-    // Crash the sandbox.
+  EXPECT_THAT(api.sum(1, 2).status(), StatusIs(absl::StatusCode::kUnavailable));
-    EXPECT_THAT(sumapi.crash(), StatusIs(absl::StatusCode::kUnavailable));
+  EXPECT_THAT(sandbox.AwaitResult().final_status(),
-    EXPECT_THAT(sumapi.sum(1, 2).status(),
+              Eq(sandbox2::Result::SIGNALED));
-                StatusIs(absl::StatusCode::kUnavailable));
-    EXPECT_THAT(sandbox->AwaitResult().final_status(),
-                Eq(sandbox2::Result::SIGNALED));
 
-    // Restart the sandbox.
+  // Restart the sandbox.
-    EXPECT_THAT(sandbox->Restart(false), IsOk());
+  ASSERT_THAT(sandbox.Restart(false), IsOk());
-    // The sandbox should now be responsive again.
-    auto result_or = sumapi.sum(1, 2);
-    EXPECT_THAT(result_or.ValueOrDie(), Eq(3));
-    return absl::OkStatus();
-  };
 
-  EXPECT_THAT(st.Run(test_body), IsOk());
+  // The sandbox should now be responsive again.
+  SAPI_ASSERT_OK_AND_ASSIGN(int result, api.sum(1, 2));
+  EXPECT_THAT(result, Eq(3));
 }
 
 TEST(SandboxTest, RestartSandboxAfterViolation) {
-  sapi::BasicTransaction st{absl::make_unique<SumSandbox>()};
+  SumSandbox sandbox;
+  ASSERT_THAT(sandbox.Init(), IsOk());
+  SumApi api(&sandbox);
 
-  auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status {
+  // Violate the sandbox policy.
-    SumApi sumapi(sandbox);
+  EXPECT_THAT(api.violate(), StatusIs(absl::StatusCode::kUnavailable));
-    // Violate the sandbox policy.
+  EXPECT_THAT(api.sum(1, 2).status(), StatusIs(absl::StatusCode::kUnavailable));
-    EXPECT_THAT(sumapi.violate(), StatusIs(absl::StatusCode::kUnavailable));
+  EXPECT_THAT(sandbox.AwaitResult().final_status(),
-    EXPECT_THAT(sumapi.sum(1, 2).status(),
+              Eq(sandbox2::Result::VIOLATION));
-                StatusIs(absl::StatusCode::kUnavailable));
-    EXPECT_THAT(sandbox->AwaitResult().final_status(),
-                Eq(sandbox2::Result::VIOLATION));
 
-    // Restart the sandbox.
+  // Restart the sandbox.
-    EXPECT_THAT(sandbox->Restart(false), IsOk());
+  ASSERT_THAT(sandbox.Restart(false), IsOk());
-    // The sandbox should now be responsive again.
-    auto result_or = sumapi.sum(1, 2);
-    EXPECT_THAT(result_or, IsOk());
-    EXPECT_THAT(result_or.ValueOrDie(), Eq(3));
-    return absl::OkStatus();
-  };
 
-  EXPECT_THAT(st.Run(test_body), IsOk());
+  // The sandbox should now be responsive again.
+  SAPI_ASSERT_OK_AND_ASSIGN(int result, api.sum(1, 2));
+  EXPECT_THAT(result, Eq(3));
 }
 
 TEST(SandboxTest, NoRaceInAwaitResult) {
-  auto sandbox = absl::make_unique<StringopSandbox>();
+  StringopSandbox sandbox;
-  ASSERT_THAT(sandbox->Init(), IsOk());
+  ASSERT_THAT(sandbox.Init(), IsOk());
-  StringopApi api(sandbox.get());
+  StringopApi api(&sandbox);
+
   EXPECT_THAT(api.violate(), StatusIs(absl::StatusCode::kUnavailable));
-  absl::SleepFor(absl::Milliseconds(200));  // make sure we lose the race
+  absl::SleepFor(absl::Milliseconds(200));  // Make sure we lose the race
-  const auto& result = sandbox->AwaitResult();
+  const auto& result = sandbox.AwaitResult();
   EXPECT_THAT(result.final_status(), Eq(sandbox2::Result::VIOLATION));
 }
 

sandboxed_api/util/status_matchers.h

@@ -29,8 +29,8 @@
 
 #define SAPI_ASSERT_OK_AND_ASSIGN_IMPL(statusor, lhs, rexpr) \
   auto statusor = (rexpr);                                   \
-  ASSERT_THAT(statusor.status(), sapi::IsOk());              \
+  ASSERT_THAT(statusor.status(), ::sapi::IsOk());            \
-  lhs = std::move(statusor).ValueOrDie();
+  lhs = std::move(statusor).value();
 
 namespace sapi {
 namespace internal {

sandboxed_api/var_proto.h

@@ -39,7 +39,7 @@ class Proto : public Pointable, public Var {
 
   ABSL_DEPRECATED("Use Proto<>::FromMessage() instead")
   explicit Proto(const T& proto)
-      : wrapped_var_(SerializeProto(proto).ValueOrDie()) {}
+      : wrapped_var_(SerializeProto(proto).value()) {}
 
   static sapi::StatusOr<Proto<T>> FromMessage(const T& proto) {
     SAPI_ASSIGN_OR_RETURN(std::vector<uint8_t> len_val, SerializeProto(proto));
@@ -67,9 +67,8 @@ class Proto : public Pointable, public Var {
 
   ABSL_DEPRECATED("Use GetMessage() instead")
   std::unique_ptr<T> GetProtoCopy() const {
-    auto result_or = GetMessage();
+    if (auto result_or = GetMessage(); result_or.ok()) {
-    if (result_or.ok()) {
+      return absl::make_unique<T>(std::move(result_or).value());
-      return absl::make_unique<T>(std::move(result_or).ValueOrDie());
     }
     return nullptr;
   }