#Cleanup method/member descriptions, build files

Drive-by:
- use `ABSL_DIE_IF_NULL` in `Sandbox2` ctor
PiperOrigin-RevId: 735721896
Change-Id: I73baf48b72e151829861b1d2738238ecfb2c92c9

Author: cblichmann

sandboxed_api/sandbox2/BUILD

@@ -327,9 +327,7 @@ cc_library(
 # sandbox2::Client objects
 cc_library(
     name = "sandbox2",
-    srcs = [
-        "sandbox2.cc",
-    ],
+    srcs = ["sandbox2.cc"],
     hdrs = [
         "client.h",
         "executor.h",
@@ -372,6 +370,7 @@ cc_library(
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/log:die_if_null",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -314,7 +314,9 @@ add_library(sandbox2_sandbox2 ${SAPI_LIB_TYPE}
 add_library(sandbox2::sandbox2 ALIAS sandbox2_sandbox2)
 target_link_libraries(sandbox2_sandbox2
   PRIVATE absl::core_headers
+          absl::check
           absl::flat_hash_set
+          absl::log
           absl::memory
           absl::optional
           absl::str_format
@@ -323,34 +325,35 @@ target_link_libraries(sandbox2_sandbox2
           sandbox2::monitor_ptrace
           sandbox2::monitor_unotify
           sapi::base
-  PUBLIC  absl::flat_hash_map
-          absl::status
-          absl::statusor
-          absl::time
-          sapi::config
-          sapi::fileops
-          sapi::temp_file
-          sandbox2::client
-          sandbox2::comms
-          sandbox2::executor
-          sandbox2::fork_client
-          sandbox2::global_forkserver
-          sandbox2::ipc
-          sandbox2::limits
-          sandbox2::logsink
-          sandbox2::monitor_base
-          sandbox2::mounts
-          sandbox2::mount_tree_proto
-          sandbox2::namespace
-          sandbox2::network_proxy_client
-          sandbox2::network_proxy_server
-          sandbox2::notify
-          sandbox2::policy
-          sandbox2::policybuilder
-          sandbox2::regs
-          sandbox2::result
-          sandbox2::syscall
-          sandbox2::util
+  PUBLIC absl::die_if_null
+         absl::flat_hash_map
+         absl::status
+         absl::statusor
+         absl::time
+         sapi::config
+         sapi::fileops
+         sapi::temp_file
+         sandbox2::client
+         sandbox2::comms
+         sandbox2::executor
+         sandbox2::fork_client
+         sandbox2::global_forkserver
+         sandbox2::ipc
+         sandbox2::limits
+         sandbox2::logsink
+         sandbox2::monitor_base
+         sandbox2::mounts
+         sandbox2::mount_tree_proto
+         sandbox2::namespace
+         sandbox2::network_proxy_client
+         sandbox2::network_proxy_server
+         sandbox2::notify
+         sandbox2::policy
+         sandbox2::policybuilder
+         sandbox2::regs
+         sandbox2::result
+         sandbox2::syscall
+         sandbox2::util
 )
 
 

sandboxed_api/sandbox2/monitor_base.cc

@@ -128,8 +128,8 @@ void LogContainer(const std::vector<std::string>& container) {
 
 MonitorBase::MonitorBase(Executor* executor, Policy* policy, Notify* notify)
     : executor_(executor),
-      notify_(notify),
       policy_(policy),
+      notify_(notify),
       // NOLINTNEXTLINE clang-diagnostic-deprecated-declarations
       comms_(executor_->ipc()->comms()),
       ipc_(executor_->ipc()),

sandboxed_api/sandbox2/monitor_base.h

@@ -80,15 +80,22 @@ class MonitorBase {
   virtual void SetWallTimeLimit(absl::Duration limit) = 0;
 
  protected:
+  // Sends the policy to the client.
+  // Can be overridden by subclasses to save/modify policy before sending.
+  // Returns success/failure status.
+  virtual absl::Status SendPolicy(const std::vector<sock_filter>& policy);
+
   bool wait_for_execveat() const { return wait_for_execveat_; }
   void set_wait_for_execveat(bool wait_for_execve) {
     wait_for_execveat_ = wait_for_execve;
   }
 
   void OnDone();
+
   // Sets basic info status and reason code in the result object.
   void SetExitStatusCode(Result::StatusEnum final_status,
                          uintptr_t reason_code);
+
   // Logs a SANDBOX VIOLATION message based on the registers and additional
   // explanation for the reason of the violation.
   void LogSyscallViolation(const Syscall& syscall) const;
@@ -108,8 +115,9 @@ class MonitorBase {
 
   // Internal objects, owned by the Sandbox2 object.
   Executor* executor_;
-  Notify* notify_;
   Policy* policy_;
+  Notify* notify_;
+
   // The sandboxee process.
   SandboxeeProcess process_;
   Result result_;
@@ -124,12 +132,6 @@ class MonitorBase {
   // Monitor type
   MonitorType type_ = FORKSERVER_MONITOR_PTRACE;
 
- protected:
-  // Sends Policy to the Client.
-  // Can be overridden by subclasses to save/modify policy before sending.
-  // Returns success/failure status.
-  virtual absl::Status SendPolicy(const std::vector<sock_filter>& policy);
-
  private:
   // Instantiates and sends Policy to the Client.
   // Returns success/failure status.

sandboxed_api/sandbox2/monitor_unotify.cc

@@ -58,10 +58,6 @@ namespace {
 
 using ::sapi::file_util::fileops::FDCloser;
 
-int seccomp(unsigned int operation, unsigned int flags, void* args) {
-  return syscall(SYS_seccomp, operation, flags, args);
-}
-
 absl::Status WaitForFdReadable(int fd, absl::Time deadline) {
   pollfd pfds[] = {
       {.fd = fd, .events = POLLIN},

sandboxed_api/sandbox2/sandbox2.h

@@ -18,13 +18,11 @@
 #ifndef SANDBOXED_API_SANDBOX2_SANDBOX2_H_
 #define SANDBOXED_API_SANDBOX2_SANDBOX2_H_
 
-#include <ctime>
 #include <memory>
 #include <utility>
 
 #include "absl/base/attributes.h"
-#include "absl/base/macros.h"
-#include "absl/log/check.h"
+#include "absl/log/die_if_null.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/time/time.h"
@@ -45,12 +43,9 @@ class Sandbox2 final {
 
   Sandbox2(std::unique_ptr<Executor> executor, std::unique_ptr<Policy> policy,
            std::unique_ptr<Notify> notify)
-      : executor_(std::move(executor)),
-        policy_(std::move(policy)),
-        notify_(std::move(notify)) {
-    CHECK(executor_ != nullptr);
-    CHECK(policy_ != nullptr);
-  }
+      : executor_(std::move(ABSL_DIE_IF_NULL(executor))),
+        policy_(std::move(ABSL_DIE_IF_NULL(policy))),
+        notify_(std::move(notify)) {}
 
   Sandbox2(const Sandbox2&) = delete;
   Sandbox2& operator=(const Sandbox2&) = delete;
@@ -66,6 +61,7 @@ class Sandbox2 final {
   // Even if set-up fails AwaitResult can still used to get a more specific
   // failure reason.
   bool RunAsync();
+
   // Waits for sandbox execution to finish and returns the execution result.
   ABSL_MUST_USE_RESULT Result AwaitResult();
 
@@ -75,8 +71,8 @@ class Sandbox2 final {
   absl::StatusOr<Result> AwaitResultWithTimeout(absl::Duration timeout);
 
   // Requests termination of the sandboxee.
-  // Sandbox should still waited with AwaitResult(), as it may finish for other
-  // reason before the request is handled.
+  // The sandbox should still waited on using AwaitResult(), as it may finish
+  // for other reasons before the request is handled.
   void Kill();
 
   // Dumps the main sandboxed process's stack trace to log.
@@ -85,7 +81,7 @@ class Sandbox2 final {
   // Returns whether sandboxing task has ended.
   bool IsTerminated() const;
 
-  // Sets a wall time limit on a running sandboxee, absl::ZeroDuration() to
+  // Sets a wall time limit on a running sandboxee. Use absl::ZeroDuration() to
   // disarm. This can be useful in a persistent sandbox scenario, to impose a
   // deadline for responses after each request and reset the deadline in
   // between. Sandboxed API can be used to implement persistent sandboxes.
@@ -94,7 +90,7 @@ class Sandbox2 final {
   // Returns the process id inside the executor.
   pid_t pid() const { return monitor_ != nullptr ? monitor_->pid() : -1; }
 
-  // Gets the comms inside the executor.
+  // Returns the comms object from the executor.
   Comms* comms() {
     return executor_ != nullptr ? executor_->ipc()->comms() : nullptr;
   }
@@ -107,16 +103,9 @@ class Sandbox2 final {
 
   std::unique_ptr<MonitorBase> CreateMonitor();
 
-  // Executor set by user - owned by Sandbox2.
   std::unique_ptr<Executor> executor_;
-
-  // Seccomp policy set by the user - owned by Sandbox2.
-  std::unique_ptr<Policy> policy_;
-
-  // Notify object - owned by Sandbox2.
+  std::unique_ptr<Policy> policy_;  // Seccomp user policy
   std::unique_ptr<Notify> notify_;
-
-  // Monitor object - owned by Sandbox2.
   std::unique_ptr<MonitorBase> monitor_;
 
   bool use_unotify_monitor_ = false;