From 0413a0d3a7e633c204881a12b506af98e97f8b3f Mon Sep 17 00:00:00 2001
From: Privacy Sandbox Team <no-reply@google.com>
Date: Wed, 6 Nov 2024 21:33:00 -0500
Subject: [PATCH] chore: Minor tidying in BYOB Roma service

Bug: N/A
Change-Id: If82fa30e659021902aa2482e5252d73c8f7968f6
GitOrigin-RevId: c18dab2bf0679d56b740f0b542837b9b61f5f928
---
 src/roma/byob/interface/roma_service.cc | 20 +++++++++++---------
 src/roma/byob/interface/roma_service.h  |  8 ++++----
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/roma/byob/interface/roma_service.cc b/src/roma/byob/interface/roma_service.cc
index 7054116c..db076fec 100644
--- a/src/roma/byob/interface/roma_service.cc
+++ b/src/roma/byob/interface/roma_service.cc
@@ -32,7 +32,7 @@
 namespace privacy_sandbox::server_common::byob::internal::roma_service {
 
 LocalHandle::LocalHandle(int pid, std::string_view mounts,
-                         std::string_view socket_path, std::string_view logdir)
+                         std::string_view socket_path, std::string_view log_dir)
     : pid_(pid) {
   // The following block does not run in the parent process.
   if (pid_ == 0) {
@@ -43,7 +43,7 @@ LocalHandle::LocalHandle(int pid, std::string_view mounts,
         absl::StrCat("--mounts=", mounts.empty() ? LIB_MOUNTS : mounts);
     const std::string socket_name_flag =
         absl::StrCat("--socket_name=", socket_path);
-    const std::string log_dir_flag = absl::StrCat("--log_dir=", logdir);
+    const std::string log_dir_flag = absl::StrCat("--log_dir=", log_dir);
     const char* argv[] = {
         run_workers_path.c_str(),
         mounts_flag.c_str(),
@@ -64,27 +64,29 @@ LocalHandle::~LocalHandle() {
 ByobHandle::ByobHandle(int pid, std::string_view mounts,
                        std::string_view socket_path,
                        std::string_view socket_dir, std::string container_name,
-                       std::string_view logdir)
+                       std::string_view log_dir)
     : pid_(pid),
       container_name_(container_name.empty() ? "default_roma_container_name"
                                              : std::move(container_name)) {
   // The following block does not run in the parent process.
   if (pid_ == 0) {
+    std::filesystem::path container_path =
+        std::filesystem::path(CONTAINER_PATH) / "config.json";
     PCHECK(::close(STDIN_FILENO) == 0);
     nlohmann::json config;
     {
-      std::ifstream ifs(std::filesystem::path(CONTAINER_PATH) / "config.json");
+      std::ifstream ifs(container_path);
       config =
           nlohmann::json::parse(std::string(std::istreambuf_iterator<char>(ifs),
                                             std::istreambuf_iterator<char>()));
     }
-    std::string logdir_mount_point = "/tmp/udf_logs";
+    constexpr std::string_view log_dir_mount_point = "/tmp/udf_logs";
     config["root"] = {{"path", CONTAINER_ROOT_RELPATH}};
     config["process"]["args"] = {
         "server/bin/run_workers",
         absl::StrCat("--mounts=", mounts.empty() ? LIB_MOUNTS : mounts),
         absl::StrCat("--socket_name=", socket_path),
-        absl::StrCat("--log_dir=", logdir_mount_point),
+        absl::StrCat("--log_dir=", log_dir_mount_point),
     };
     config["mounts"] = {
         {
@@ -94,14 +96,14 @@ ByobHandle::ByobHandle(int pid, std::string_view mounts,
             {"options", {"rbind", "rprivate"}},
         },
         {
-            {"destination", logdir_mount_point},
+            {"destination", log_dir_mount_point},
             {"type", "bind"},
-            {"source", logdir},
+            {"source", log_dir},
             {"options", {"rbind", "rprivate"}},
         },
     };
     {
-      std::ofstream ofs(std::filesystem::path(CONTAINER_PATH) / "config.json");
+      std::ofstream ofs(container_path);
       ofs << config.dump();
     }
     PCHECK(::chdir(CONTAINER_PATH) == 0);
diff --git a/src/roma/byob/interface/roma_service.h b/src/roma/byob/interface/roma_service.h
index 827c504a..71bd9c8f 100644
--- a/src/roma/byob/interface/roma_service.h
+++ b/src/roma/byob/interface/roma_service.h
@@ -55,7 +55,7 @@ namespace internal::roma_service {
 class LocalHandle final {
  public:
   LocalHandle(int pid, std::string_view mounts, std::string_view socket_path,
-              std::string_view logdir);
+              std::string_view log_dir);
   ~LocalHandle();
 
  private:
@@ -66,7 +66,7 @@ class ByobHandle final {
  public:
   ByobHandle(int pid, std::string_view mounts, std::string_view socket_path,
              std::string_view sockdir, std::string container_name,
-             std::string_view logdir);
+             std::string_view log_dir);
   ~ByobHandle();
 
  private:
@@ -86,7 +86,7 @@ class RomaService final {
   absl::Status Init(Config<TMetadata> config, Mode mode) {
     char socket_dir_tmpl[20] = "/tmp/sockdir_XXXXXX";
     if (::mkdtemp(socket_dir_tmpl) == nullptr) {
-      return absl::ErrnoToStatus(errno, "mkdtemp(\"/tmp/sockdir_XXXXXX\")");
+      return absl::ErrnoToStatus(errno, "mkdtemp(socket_dir)");
     }
     socket_dir_ = socket_dir_tmpl;
     const int fd = ::socket(AF_UNIX, SOCK_STREAM, 0);
@@ -119,7 +119,7 @@ class RomaService final {
                                      std::filesystem::perms::others_write);
     char log_dir_tmpl[20] = "/tmp/log_dir_XXXXXX";
     if (::mkdtemp(log_dir_tmpl) == nullptr) {
-      return absl::ErrnoToStatus(errno, "mkdtemp(\"/tmp/log_dir_XXXXXX\")");
+      return absl::ErrnoToStatus(errno, "mkdtemp(log_dir)");
     }
     log_dir_ = log_dir_tmpl;
     std::filesystem::permissions(log_dir_,