Introduce sapi::OsErrorMessage() for error handling

This should make handling OS error less repetetive.

PiperOrigin-RevId: 387074642
Change-Id: I09b8c5e37e7f7b08341e22ba01ccda21a916a4bc

sandboxed_api/sandbox2/BUILD.bazel

@@ -45,6 +45,7 @@ cc_library(
         ":syscall",
         ":violation_cc_proto",
         "//sandboxed_api:config",
+        "//sandboxed_api/util:status",
         "//sandboxed_api/util:strerror",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/status",
@@ -410,6 +411,7 @@ cc_library(
         "//sandboxed_api/util:file_helpers",
         "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:raw_logging",
+        "//sandboxed_api/util:status",
         "//sandboxed_api/util:strerror",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/memory",
@@ -550,8 +552,9 @@ cc_library(
         "//sandboxed_api/util:file_helpers",
         "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:raw_logging",
-        "//sandboxed_api/util:strerror",
+        "//sandboxed_api/util:status",
         "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
@@ -566,9 +569,10 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":util",
-        "//sandboxed_api/util:strerror",
+        "//sandboxed_api/util:status",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/memory",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
     ],

sandboxed_api/sandbox2/CMakeLists.txt

@@ -498,7 +498,7 @@ target_link_libraries(sandbox2_util
           sapi::file_base
           sapi::file_helpers
           sapi::fileops
-          sapi::strerror
+          sapi::status
           sapi::base
           sapi::raw_logging
   PUBLIC absl::status

sandboxed_api/sandbox2/buffer.cc

@@ -21,15 +21,14 @@
 #include <cerrno>
 
 #include "absl/memory/memory.h"
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/sandbox2/util.h"
-#include "sandboxed_api/util/strerror.h"
+#include "sandboxed_api/util/os_error.h"
 
 namespace sandbox2 {
 
-using ::sapi::StrError;
-
 // Creates a new Buffer that is backed by the specified file descriptor.
 absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateFromFd(int fd) {
   auto buffer = absl::WrapUnique(new Buffer{});
@@ -37,7 +36,7 @@ absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateFromFd(int fd) {
   struct stat stat_buf;
   if (fstat(fd, &stat_buf) != 0) {
     return absl::InternalError(
-        absl::StrCat("Could not stat buffer fd: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Could not stat buffer fd"));
   }
   size_t size = stat_buf.st_size;
   int prot = PROT_READ | PROT_WRITE;
@@ -47,7 +46,7 @@ absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateFromFd(int fd) {
       reinterpret_cast<uint8_t*>(mmap(nullptr, size, prot, flags, fd, offset));
   if (buffer->buf_ == MAP_FAILED) {
     return absl::InternalError(
-        absl::StrCat("Could not map buffer fd: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Could not map buffer fd"));
   }
   buffer->fd_ = fd;
   buffer->size_ = size;
@@ -63,7 +62,7 @@ absl::StatusOr<std::unique_ptr<Buffer>> Buffer::CreateWithSize(size_t size) {
   }
   if (ftruncate(fd, size) != 0) {
     return absl::InternalError(
-        absl::StrCat("Could not extend buffer fd: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Could not extend buffer fd"));
   }
   return CreateFromFd(fd);
 }

sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc

@@ -20,8 +20,8 @@
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/network_proxy/client.h"
 #include "sandboxed_api/util/fileops.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/util/strerror.h"
 
 ABSL_FLAG(bool, connect_with_handler, true, "Connect using automatic mode.");
 
@@ -66,8 +66,7 @@ absl::StatusOr<struct sockaddr_in6> CreateAddres(int port) {
 
   int err = inet_pton(AF_INET6, "::1", &saddr.sin6_addr);
   if (err <= 0) {
-    return absl::InternalError(
+    return absl::InternalError(sapi::OsErrorMessage(errno, "socket() failed"));
-        absl::StrCat("socket() failed: ", sapi::StrError(errno)));
   }
   return saddr;
 }
@@ -92,8 +91,7 @@ absl::StatusOr<int> ConnectToServer(int port) {
 
   sapi::file_util::fileops::FDCloser s(socket(AF_INET6, SOCK_STREAM, 0));
   if (s.get() < 0) {
-    return absl::InternalError(
+    return absl::InternalError(sapi::OsErrorMessage(errno, "socket() failed"));
-        absl::StrCat("socket() failed: ", sapi::StrError(errno)));
   }
 
   if (absl::GetFlag(FLAGS_connect_with_handler)) {

sandboxed_api/sandbox2/forkserver.cc

@@ -55,6 +55,7 @@
 #include "sandboxed_api/sandbox2/util.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
 #include "sandboxed_api/util/fileops.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/raw_logging.h"
 #include "sandboxed_api/util/strerror.h"
 
@@ -168,7 +169,7 @@ absl::Status SendPid(int signaling_fd) {
   char dummy = ' ';
   if (TEMP_FAILURE_RETRY(send(signaling_fd, &dummy, 1, 0)) != 1) {
     return absl::InternalError(
-        absl::StrCat("Sending PID: send: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Sending PID: send()"));
   }
   return absl::OkStatus();
 }
@@ -193,7 +194,7 @@ absl::StatusOr<pid_t> ReceivePid(int signaling_fd) {
 
   if (TEMP_FAILURE_RETRY(recvmsg(signaling_fd, &msgh, MSG_WAITALL)) != 1) {
     return absl::InternalError(
-        absl::StrCat("Receiving pid failed: recvmsg: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Receiving pid failed: recvmsg"));
   }
   struct cmsghdr* cmsgp = CMSG_FIRSTHDR(&msgh);
   if (cmsgp->cmsg_len != CMSG_LEN(sizeof(struct ucred)) ||

sandboxed_api/sandbox2/network_proxy/client.cc

@@ -28,13 +28,11 @@
 #include "absl/status/status.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/config.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/util/strerror.h"
 
 namespace sandbox2 {
 
-using ::sapi::StrError;
-
 #ifndef SYS_SECCOMP
 constexpr int SYS_SECCOMP = 1;
 #endif
@@ -123,7 +121,7 @@ absl::Status NetworkProxyClient::ReceiveRemoteResult() {
   if (result != 0) {
     errno = result;
     return absl::InternalError(
-        absl::StrCat("Error in network proxy server: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Error in network proxy server"));
   }
   return absl::OkStatus();
 }

sandboxed_api/sandbox2/network_proxy/filtering.cc

@@ -22,13 +22,11 @@
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_split.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/util/strerror.h"
 
 namespace sandbox2 {
 
-using ::sapi::StrError;
-
 static absl::StatusOr<std::string> Addr6ToString(
     const struct sockaddr_in6* saddr) {
   char addr[INET6_ADDRSTRLEN];
@@ -72,8 +70,8 @@ static absl::Status IPStringToAddr(const std::string& ip, int address_family,
     return absl::InvalidArgumentError(absl::StrCat("Invalid address: ", ip));
   }
   if (err == -1) {
-    return absl::InternalError(
+    return absl::InternalError(sapi::OsErrorMessage(
-        absl::StrCat("inet_pton() failed for ", ip, ": ", StrError(errno)));
+        errno, absl::StrCat("inet_pton() failed for ", ip)));
   }
 
   return absl::OkStatus();

sandboxed_api/sandbox2/regs.cc

@@ -27,12 +27,10 @@
 #include "absl/status/status.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/config.h"
-#include "sandboxed_api/util/strerror.h"
+#include "sandboxed_api/util/os_error.h"
 
 namespace sandbox2 {
 
-using ::sapi::StrError;
-
 #ifndef NT_ARM_SYSTEM_CALL
 #define NT_ARM_SYSTEM_CALL 0x404
 #endif
@@ -40,8 +38,8 @@ using ::sapi::StrError;
 absl::Status Regs::Fetch() {
 #ifdef SAPI_X86_64
   if (ptrace(PTRACE_GETREGS, pid_, 0, &user_regs_) == -1L) {
-    return absl::InternalError(absl::StrCat("ptrace(PTRACE_GETREGS, pid=", pid_,
+    return absl::InternalError(sapi::OsErrorMessage(
-                                            ") failed: ", StrError(errno)));
+        errno, absl::StrCat("ptrace(PTRACE_GETREGS, pid=", pid_, ") failed")));
   }
 #endif
   if constexpr (sapi::host_cpu::IsPPC64LE() || sapi::host_cpu::IsArm64() ||
@@ -49,9 +47,9 @@ absl::Status Regs::Fetch() {
     iovec pt_iov = {&user_regs_, sizeof(user_regs_)};
 
     if (ptrace(PTRACE_GETREGSET, pid_, NT_PRSTATUS, &pt_iov) == -1L) {
-      return absl::InternalError(
+      return absl::InternalError(sapi::OsErrorMessage(
-          absl::StrCat("ptrace(PTRACE_GETREGSET, pid=", pid_,
+          errno,
-                       ") failed: ", StrError(errno)));
+          absl::StrCat("ptrace(PTRACE_GETREGSET, pid=", pid_, ") failed")));
     }
     if (pt_iov.iov_len != sizeof(user_regs_)) {
       return absl::InternalError(absl::StrCat(
@@ -65,9 +63,9 @@ absl::Status Regs::Fetch() {
       iovec sys_iov = {&syscall_number_, sizeof(syscall_number_)};
 
       if (ptrace(PTRACE_GETREGSET, pid_, NT_ARM_SYSTEM_CALL, &sys_iov) == -1L) {
-        return absl::InternalError(
+        return absl::InternalError(sapi::OsErrorMessage(
-            absl::StrCat("ptrace(PTRACE_GETREGSET, pid=", pid_,
+            errno, absl::StrCat("ptrace(PTRACE_GETREGSET, pid=", pid_,
-                         ", NT_ARM_SYSTEM_CALL) failed: ", StrError(errno)));
+                                ", NT_ARM_SYSTEM_CALL)")));
       }
       if (sys_iov.iov_len != sizeof(syscall_number_)) {
         return absl::InternalError(absl::StrCat(
@@ -84,8 +82,8 @@ absl::Status Regs::Fetch() {
 absl::Status Regs::Store() {
 #ifdef SAPI_X86_64
   if (ptrace(PTRACE_SETREGS, pid_, 0, &user_regs_) == -1) {
-    return absl::InternalError(absl::StrCat("ptrace(PTRACE_SETREGS, pid=", pid_,
+    return absl::InternalError(sapi::OsErrorMessage(
-                                            ") failed: ", StrError(errno)));
+        errno, absl::StrCat("ptrace(PTRACE_SETREGS, pid=", pid_, ")")));
   }
 #endif
   if constexpr (sapi::host_cpu::IsPPC64LE() || sapi::host_cpu::IsArm64() ||
@@ -93,9 +91,9 @@ absl::Status Regs::Store() {
     iovec pt_iov = {&user_regs_, sizeof(user_regs_)};
 
     if (ptrace(PTRACE_SETREGSET, pid_, NT_PRSTATUS, &pt_iov) == -1L) {
-      return absl::InternalError(
+      return absl::InternalError(sapi::OsErrorMessage(
-          absl::StrCat("ptrace(PTRACE_SETREGSET, pid=", pid_,
+          errno,
-                       ") failed: ", StrError(errno)));
+          absl::StrCat("ptrace(PTRACE_SETREGSET, pid=", pid_, ") failed")));
     }
 
     // Store syscall number on AArch64.
@@ -103,9 +101,9 @@ absl::Status Regs::Store() {
       iovec sys_iov = {&syscall_number_, sizeof(syscall_number_)};
 
       if (ptrace(PTRACE_SETREGSET, pid_, NT_ARM_SYSTEM_CALL, &sys_iov) == -1L) {
-        return absl::InternalError(
+        return absl::InternalError(sapi::OsErrorMessage(
-            absl::StrCat("ptrace(PTRACE_SETREGSET, pid=", pid_,
+            errno, absl::StrCat("ptrace(PTRACE_SETREGSET, pid=", pid_,
-                         ", NT_ARM_SYSTEM_CALL) failed: ", StrError(errno)));
+                                ", NT_ARM_SYSTEM_CALL) failed")));
       }
     }
   }

sandboxed_api/sandbox2/sanitizer.cc

@@ -47,9 +47,7 @@
 namespace sandbox2::sanitizer {
 namespace {
 
-namespace file = ::sapi::file;
 namespace file_util = ::sapi::file_util;
-using ::sapi::StrError;
 
 constexpr char kProcSelfFd[] = "/proc/self/fd";
 

sandboxed_api/sandbox2/util.cc

@@ -34,6 +34,7 @@
 #include <memory>
 
 #include "absl/base/attributes.h"
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/ascii.h"
 #include "absl/strings/escaping.h"
@@ -47,15 +48,14 @@
 #include "sandboxed_api/config.h"
 #include "sandboxed_api/util/file_helpers.h"
 #include "sandboxed_api/util/fileops.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/path.h"
 #include "sandboxed_api/util/raw_logging.h"
-#include "sandboxed_api/util/strerror.h"
 
 namespace sandbox2::util {
 
 namespace file = ::sapi::file;
 namespace file_util = ::sapi::file_util;
-using ::sapi::StrError;
 
 void CharPtrArrToVecString(char* const* arr, std::vector<std::string>* vec) {
   for (int i = 0; arr[i]; ++i) {
@@ -235,7 +235,7 @@ absl::StatusOr<int> Communicate(const std::vector<std::string>& argv,
   posix_spawn_file_actions_t action;
 
   if (pipe(cout_pipe) == -1) {
-    return absl::UnknownError(absl::StrCat("creating pipe: ", StrError(errno)));
+    return absl::UnknownError(sapi::OsErrorMessage(errno, "creating pipe"));
   }
   file_util::fileops::FDCloser cout_closer{cout_pipe[1]};
 
@@ -264,8 +264,7 @@ absl::StatusOr<int> Communicate(const std::vector<std::string>& argv,
 
   pid_t pid;
   if (posix_spawnp(&pid, args[0], &action, nullptr, args, envp) != 0) {
-    return absl::UnknownError(
+    return absl::UnknownError(sapi::OsErrorMessage(errno, "posix_spawnp()"));
-        absl::StrCat("posix_spawnp() failed: ", StrError(errno)));
   }
 
   // Close child end of the pipe.
@@ -277,7 +276,7 @@ absl::StatusOr<int> Communicate(const std::vector<std::string>& argv,
         TEMP_FAILURE_RETRY(read(cout_pipe[0], &buffer[0], buffer.length()));
     if (bytes_read < 0) {
       return absl::InternalError(
-          absl::StrCat("reading from cout pipe failed: ", StrError(errno)));
+          sapi::OsErrorMessage(errno, "reading from cout pipe"));
     }
     if (bytes_read == 0) {
       break;  // Nothing left to read
@@ -379,9 +378,10 @@ absl::StatusOr<std::string> ReadCPathFromPid(pid_t pid, uintptr_t ptr) {
   ssize_t sz = process_vm_readv(pid, local_iov, ABSL_ARRAYSIZE(local_iov),
                                 remote_iov, ABSL_ARRAYSIZE(remote_iov), 0);
   if (sz < 0) {
-    return absl::InternalError(absl::StrFormat(
+    return absl::InternalError(sapi::OsErrorMessage(
-        "process_vm_readv() failed for PID: %d at address: %#x: %s", pid,
+        errno,
-        reinterpret_cast<uintptr_t>(ptr), StrError(errno)));
+        absl::StrFormat("process_vm_readv() failed for PID: %d at address: %#x",
+                        pid, reinterpret_cast<uintptr_t>(ptr))));
   }
 
   // Check for whether there's a NUL byte in the buffer. If not, it's an

sandboxed_api/sandbox2/util/minielf.cc

@@ -26,12 +26,11 @@
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/config.h"
 #include "sandboxed_api/sandbox2/util.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/raw_logging.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/util/strerror.h"
 
 namespace host_cpu = ::sapi::host_cpu;
-using ::sapi::StrError;
 
 namespace sandbox2 {
 
@@ -63,7 +62,7 @@ namespace {
 absl::Status CheckedFSeek(FILE* f, long offset, int whence) {
   if (fseek(f, offset, whence)) {
     return absl::FailedPreconditionError(
-        absl::StrCat("Fseek on ELF failed: ", StrError(errno)));
+        sapi::OsErrorMessage(errno, "Fseek on ELF failed"));
   }
   return absl::OkStatus();
 }
@@ -73,7 +72,7 @@ absl::Status CheckedFRead(void* dst, size_t size, size_t nmemb, FILE* f) {
     return absl::OkStatus();
   }
   return absl::FailedPreconditionError(
-      absl::StrCat("Reading ELF data failed: ", StrError(errno)));
+      sapi::OsErrorMessage(errno, "Reading ELF data failed"));
 }
 
 absl::Status CheckedRead(std::string* s, FILE* f) {
@@ -470,8 +469,8 @@ absl::StatusOr<ElfFile> ElfParser::Parse(const std::string& filename,
                                          uint32_t features) {
   ElfParser parser;
   if (parser.elf_ = std::fopen(filename.c_str(), "r"); !parser.elf_) {
-    return absl::UnknownError(
+    return absl::UnknownError(sapi::OsErrorMessage(
-        absl::StrCat("cannot open file: ", filename, ": ", StrError(errno)));
+        errno, absl::StrCat("cannot open file: ", filename)));
   }
 
   // Basic sanity check.

sandboxed_api/util/BUILD.bazel

@@ -143,11 +143,12 @@ sapi_proto_library(
     srcs = ["status.proto"],
 )
 
-# Reimplementations of utility functions not released with absl::Status.
+# Implementations of utility functions not released with absl::Status.
 cc_library(
     name = "status",
     srcs = ["status.cc"],
     hdrs = [
+        "os_error.h",
         "status.h",
         "status_macros.h",
     ],
@@ -155,6 +156,7 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":status_cc_proto",
+        ":strerror",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",

sandboxed_api/util/CMakeLists.txt

@@ -105,6 +105,7 @@ target_link_libraries(sapi_util_status_proto
 
 # sandboxed_api/util:status
 add_library(sapi_util_status ${SAPI_LIB_TYPE}
+  os_error.h
   status.cc
   status.h
   status_macros.h
@@ -115,6 +116,7 @@ target_link_libraries(sapi_util_status
           absl::status
           absl::strings
           sapi::base
+          sapi::strerror
           protobuf::libprotobuf
   PUBLIC sapi::status_proto
 )

sandboxed_api/util/os_error.h

@@ -0,0 +1,36 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SANDBOXED_API_UTIL_OS_ERROR_H_
+#define SANDBOXED_API_UTIL_OS_ERROR_H_
+
+#include <utility>
+
+#include "absl/strings/str_cat.h"
+#include "sandboxed_api/util/strerror.h"
+
+namespace sapi {
+
+// Returns a
+// `ErrnoToCanonicalCode(error_number)`, and a `message` with the result of
+// `StrError(error_number)` appended.
+template <typename... Arg>
+std::string OsErrorMessage(int error_number, const Arg&... args) {
+  return absl::StrCat(std::forward<const Arg&>(args)..., ": ",
+                      ::sapi::StrError(error_number));
+}
+
+}  // namespace sapi
+
+#endif  // SANDBOXED_API_UTIL_OS_ERROR_H_

sandboxed_api/util/temp_file.cc

@@ -22,11 +22,12 @@
 #include <cstdlib>
 #include <vector>
 
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/util/fileops.h"
+#include "sandboxed_api/util/os_error.h"
 #include "sandboxed_api/util/status_macros.h"
-#include "sandboxed_api/util/strerror.h"
 
 namespace sapi {
 
@@ -39,7 +40,7 @@ absl::StatusOr<std::pair<std::string, int>> CreateNamedTempFile(
   std::string name_template = absl::StrCat(prefix, kMktempSuffix);
   int fd = mkstemp(&name_template[0]);
   if (fd < 0) {
-    return absl::UnknownError(absl::StrCat("mkstemp(): ", StrError(errno)));
+    return absl::UnknownError(sapi::OsErrorMessage(errno, "mkstemp()"));
   }
   return std::pair<std::string, int>{std::move(name_template), fd};
 }
@@ -54,7 +55,7 @@ absl::StatusOr<std::string> CreateNamedTempFileAndClose(
 absl::StatusOr<std::string> CreateTempDir(absl::string_view prefix) {
   std::string name_template = absl::StrCat(prefix, kMktempSuffix);
   if (mkdtemp(&name_template[0]) == nullptr) {
-    return absl::UnknownError(absl::StrCat("mkdtemp(): ", StrError(errno)));
+    return absl::UnknownError(sapi::OsErrorMessage(errno, "mkdtemp()"));
   }
   return name_template;
 }