dissolve launcher_messager.h into command_util/util.h

These are two APIs for the same thing. When launcher_message is inlined
into the util.h functions, many things can be simplified.

The read/write code was tweaked a bit. There is less copying when
writing. When reading, the old code didn't check the bytes read, which,
surprisingly, is needed when calling the ReadExact{,Binary} functions.

Test: launch CF
Change-Id: Iddd0a01c2b491df40159d22983c749772430b7f4

4 files changed, 91 insertions, 244 deletions

diff --git a/host/libs/command_util/Android.bp b/host/libs/command_util/Android.bp
index 0ee91d27d..056d4dfc5 100644
--- a/host/libs/command_util/Android.bp
+++ b/host/libs/command_util/Android.bp
@@ -20,7 +20,6 @@ package {
 cc_library {
     name: "libcuttlefish_command_util",
     srcs: [
-        "launcher_message.cc",
         "snapshot_utils.cc",
         "util.cc",
     ],
diff --git a/host/libs/command_util/launcher_message.cc b/host/libs/command_util/launcher_message.cc
deleted file mode 100644
index 5ef35b2a5..000000000
--- a/host/libs/command_util/launcher_message.cc
+++ /dev/null
@@ -1,151 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * 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.
- */
-
-#include "host/libs/command_util/launcher_message.h"
-
-#include <cstdint>
-#include <set>
-#include <string>
-
-#include <fmt/core.h>
-
-#include "common/libs/fs/shared_buf.h"
-#include "common/libs/fs/shared_fd.h"
-#include "common/libs/utils/contains.h"
-#include "common/libs/utils/result.h"
-
-namespace cuttlefish {
-namespace run_cvd_msg_impl {
-
-Result<LauncherActionMessage> LauncherActionMessage::Create(
-    const LauncherAction action) {
-  CF_EXPECTF(IsShortAction(action),
-             "LauncherAction {} is not supported by legacy "
-             "LauncherActionMessage::Create()",
-             static_cast<const char>(action));
-  auto action_message = CF_EXPECT(
-      LauncherActionMessage::Create(action, ExtendedActionType::kUnused, ""));
-  return action_message;
-}
-
-Result<LauncherActionMessage> LauncherActionMessage::Create(
-    const LauncherAction action, const ExtendedActionType type,
-    std::string serialized_data) {
-  if (IsShortAction(action)) {
-    const char action_char = static_cast<const char>(action);
-    CF_EXPECTF(
-        type == ExtendedActionType::kUnused,
-        "The type of action \"{}\" should be ExtendedActionType::kUnused",
-        action_char);
-    if (!serialized_data.empty()) {
-      LOG(DEBUG) << "serialized_data is ignored when the action is \""
-                 << action_char << "\"";
-    }
-  }
-  LauncherActionMessage new_message(action, type, std::move(serialized_data));
-  return new_message;
-}
-
-LauncherActionMessage::LauncherActionMessage(const LauncherAction action,
-                                             const ExtendedActionType type,
-                                             std::string serialized_data)
-    : action_(action),
-      type_(type),
-      serialized_data_(std::move(serialized_data)) {}
-
-bool LauncherActionMessage::IsShortAction(const LauncherAction action) {
-  std::set<LauncherAction> supported_actions{
-      LauncherAction::kPowerwash,
-      LauncherAction::kRestart,
-      LauncherAction::kStatus,
-      LauncherAction::kStop,
-  };
-  return Contains(supported_actions, action);
-}
-
-static Result<void> WriteBuffer(const SharedFD& fd, const std::string& buf,
-                                const std::string& description) {
-  CF_EXPECT(fd->IsOpen(), "The file descriptor to write is not open.");
-  ssize_t bytes_sent = WriteAll(fd, buf);
-  CF_EXPECTF(bytes_sent > 0, "Error sending {} to launcher monitor: {}",
-             description, fd->StrError());
-  CF_EXPECTF(bytes_sent == buf.size(), "LauncherActionMessage::WriteToFd() ",
-             "did not send correct number of bytes");
-  return {};
-}
-
-// use this when sizeof(T) is the number of bytes we want to send
-template <typename T>
-static Result<void> WriteSizeOfT(const SharedFD& fd, const T& t,
-                                 const std::string& description) {
-  CF_EXPECT(fd->IsOpen(), "The file descriptor to write is not open.");
-  const char* start_addr = reinterpret_cast<const char*>(&t);
-  std::vector<char> tmp{start_addr, start_addr + sizeof(T)};
-  std::string buf{tmp.begin(), tmp.end()};
-  CF_EXPECT(WriteBuffer(fd, buf, description));
-  return {};
-}
-
-Result<void> LauncherActionMessage::WriteToFd(const SharedFD& fd) {
-  CF_EXPECT(WriteSizeOfT(fd, action_, "LauncherAction"));
-  if (IsShortAction(action_)) {
-    return {};
-  }
-  CF_EXPECT(WriteSizeOfT(fd, type_, "ExtendedActionType"));
-  const SerializedDataSizeType length = serialized_data_.size();
-  CF_EXPECT(WriteSizeOfT(fd, length, "Length of serialized data"));
-  if (!serialized_data_.empty()) {
-    CF_EXPECT(WriteBuffer(fd, serialized_data_, "serialized data"));
-  }
-  return {};
-}
-
-Result<LauncherActionMessage> LauncherActionMessage::ReadFromFd(
-    const SharedFD& fd) {
-  CF_EXPECT(fd->IsOpen(), "The file descriptor for ReadFromFd is not open.");
-  LauncherAction action;
-  ssize_t n_bytes = 0;
-  CF_EXPECTF((n_bytes = fd->Read(&action, sizeof(action))) > 0,
-             "The returned n_bytes is not {} but {}: {}", sizeof(action),
-             n_bytes, fd->StrError());
-  if (IsShortAction(action)) {
-    return CF_EXPECT(LauncherActionMessage::Create(action));
-  }
-  ExtendedActionType type;
-  CF_EXPECTF((n_bytes = fd->Read(&type, sizeof(type))) > 0,
-             "The returned n_bytes is not {} but {}", sizeof(type), n_bytes);
-  SerializedDataSizeType length = 0;
-  CF_EXPECTF((n_bytes = fd->Read(&length, sizeof(length))) > 0,
-             "The returned n_bytes is not {} but {}", sizeof(length), n_bytes);
-  if (length == 0) {
-    return CF_EXPECT(LauncherActionMessage::Create(action, type, ""));
-  }
-  std::string serialized_data(length, 0);
-  CF_EXPECTF((n_bytes = ReadExact(fd, &serialized_data)) > 0,
-             "The returned n_bytes is not {} but {}", sizeof(length), n_bytes);
-  auto message =
-      CF_EXPECT(LauncherActionMessage::Create(action, type, serialized_data));
-  return message;
-}
-
-LauncherAction LauncherActionMessage::Action() const { return action_; }
-ExtendedActionType LauncherActionMessage::Type() const { return type_; }
-const std::string& LauncherActionMessage::SerializedData() const {
-  return serialized_data_;
-}
-
-}  // namespace run_cvd_msg_impl
-}  // namespace cuttlefish
diff --git a/host/libs/command_util/launcher_message.h b/host/libs/command_util/launcher_message.h
deleted file mode 100644
index 95d5d886f..000000000
--- a/host/libs/command_util/launcher_message.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * 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.
- */
-
-#pragma once
-
-#include <cstdint>
-#include <string>
-
-#include "common/libs/fs/shared_buf.h"
-#include "common/libs/fs/shared_fd.h"
-#include "common/libs/utils/contains.h"
-#include "common/libs/utils/result.h"
-#include "host/libs/command_util/runner/defs.h"
-
-namespace cuttlefish {
-namespace run_cvd_msg_impl {
-
-class LauncherActionMessage {
- public:
-  using SerializedDataSizeType = std::uint32_t;
-  /**
-   * supported for backward compatibility, so only the following are accepted:
-   *  kPowerwash, kRestart, kStatus, kStop
-   */
-  static Result<LauncherActionMessage> Create(const LauncherAction action);
-  // use std::move(serialized_data) to avoid copy the buffer
-  static Result<LauncherActionMessage> Create(const LauncherAction action,
-                                              const ExtendedActionType type,
-                                              std::string serialized_data);
-  Result<void> WriteToFd(const SharedFD& fd);
-  static Result<LauncherActionMessage> ReadFromFd(const SharedFD& fd);
-
-  LauncherAction Action() const;
-  ExtendedActionType Type() const;
-  const std::string& SerializedData() const;
-
- private:
-  LauncherActionMessage(const LauncherAction action,
-                        const ExtendedActionType type,
-                        std::string serialized_data);
-  // returns true if the action does not need extended field
-  static bool IsShortAction(const LauncherAction action);
-
-  const LauncherAction action_;
-  const ExtendedActionType type_;
-  // mostly for protobuf message
-  const std::string serialized_data_;
-};
-
-}  // namespace run_cvd_msg_impl
-}  // namespace cuttlefish
diff --git a/host/libs/command_util/util.cc b/host/libs/command_util/util.cc
index 6a1322075..8b4f4534b 100644
--- a/host/libs/command_util/util.cc
+++ b/host/libs/command_util/util.cc
@@ -25,31 +25,47 @@
 #include "common/libs/fs/shared_fd.h"
 #include "common/libs/fs/shared_select.h"
 #include "common/libs/utils/result.h"
-#include "host/libs/command_util/launcher_message.h"
 #include "host/libs/command_util/runner/defs.h"
 #include "host/libs/config/cuttlefish_config.h"
 
 namespace cuttlefish {
 namespace {
 
-using run_cvd_msg_impl::LauncherActionMessage;
+bool IsShortAction(const LauncherAction action) {
+  switch (action) {
+    case LauncherAction::kPowerwash:
+    case LauncherAction::kRestart:
+    case LauncherAction::kStatus:
+    case LauncherAction::kStop:
+      return true;
+    default:
+      return false;
+  };
+}
 
 template <typename T>
-Result<T> ReadFromMonitor(const SharedFD& monitor_socket) {
-  T response;
-  ssize_t bytes_recv = ReadExactBinary(monitor_socket, &response);
-  CF_EXPECT(bytes_recv != 0, "Launcher socket closed unexpectedly");
-  CF_EXPECT(bytes_recv > 0, "Error receiving response from launcher monitor: "
-                                << monitor_socket->StrError());
-  CF_EXPECT(bytes_recv == sizeof(response),
-            "Launcher response not correct number of bytes");
-  return response;
+static Result<void> WriteAllBinaryResult(const SharedFD& fd, const T* t) {
+  ssize_t n = WriteAllBinary(fd, t);
+  CF_EXPECTF(n > 0, "Write error: {}", fd->StrError());
+  CF_EXPECT(n == sizeof(*t), "Unexpected EOF on write");
+  return {};
+}
+
+template <typename T>
+static Result<void> ReadExactBinaryResult(const SharedFD& fd, T* t) {
+  ssize_t n = ReadExactBinary(fd, t);
+  CF_EXPECTF(n > 0, "Read error: {}", fd->StrError());
+  CF_EXPECT(n == sizeof(*t), "Unexpected EOF on read");
+  return {};
 }
 
 }  // namespace
 
 Result<RunnerExitCodes> ReadExitCode(const SharedFD& monitor_socket) {
-  return ReadFromMonitor<RunnerExitCodes>(monitor_socket);
+  RunnerExitCodes exit_codes;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &exit_codes),
+            "Error reading RunnerExitCodes");
+  return exit_codes;
 }
 
 Result<SharedFD> GetLauncherMonitorFromInstance(
@@ -75,12 +91,43 @@ Result<SharedFD> GetLauncherMonitor(const CuttlefishConfig& config,
 
 Result<LauncherActionInfo> ReadLauncherActionFromFd(
     const SharedFD& monitor_socket) {
-  using run_cvd_msg_impl::LauncherActionMessage;
-  auto message = CF_EXPECT(LauncherActionMessage::ReadFromFd(monitor_socket));
+  LauncherAction action;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &action),
+            "Error reading LauncherAction");
+  if (IsShortAction(action)) {
+    return LauncherActionInfo{
+        .action = action,
+        .type = ExtendedActionType::kUnused,
+        .serialized_data = "",
+    };
+  }
+  ExtendedActionType type;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &type),
+            "Error reading ExtendedActionType");
+  std::uint32_t length = 0;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &length),
+            "Error reading proto length");
+  if (length == 0) {
+    return LauncherActionInfo{
+        .action = action,
+        .type = type,
+        .serialized_data = "",
+    };
+  }
+  std::string serialized_data(length, 0);
+  ssize_t n =
+      ReadExact(monitor_socket, serialized_data.data(), serialized_data.size());
+  CF_EXPECTF(n > 0, "Read error: {}", monitor_socket->StrError());
+  CF_EXPECT(n == serialized_data.size(), "Unexpected EOF on read");
+
+  run_cvd::ExtendedLauncherAction extended_action;
+  CF_EXPECT(extended_action.ParseFromString(serialized_data),
+            "Failed to parse ExtendedLauncherAction proto");
+
   return LauncherActionInfo{
-      .action = message.Action(),
-      .type = message.Type(),
-      .serialized_data = message.SerializedData(),
+      .action = action,
+      .type = type,
+      .serialized_data = std::move(serialized_data),
   };
 }
 
@@ -103,14 +150,18 @@ Result<void> WaitForRead(const SharedFD& monitor_socket,
 Result<void> RunLauncherAction(const SharedFD& monitor_socket,
                                LauncherAction action,
                                std::optional<int> timeout_seconds) {
-  auto message = CF_EXPECT(
-      LauncherActionMessage::Create(action, ExtendedActionType::kUnused, ""));
-  CF_EXPECT(message.WriteToFd(monitor_socket));
+  CF_EXPECTF(IsShortAction(action),
+             "PerformActionRequest doesn't support extended action \"{}\"",
+             static_cast<const char>(action));
+  CF_EXPECT(WriteAllBinaryResult(monitor_socket, &action),
+            "Error writing LauncherAction");
+
   if (timeout_seconds.has_value()) {
     CF_EXPECT(WaitForRead(monitor_socket, timeout_seconds.value()));
   }
-  LauncherResponse response =
-      CF_EXPECT(ReadFromMonitor<LauncherResponse>(monitor_socket));
+  LauncherResponse response;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &response),
+            "Error reading LauncherResponse");
   CF_EXPECT_EQ((int)response, (int)LauncherResponse::kSuccess);
   return {};
 }
@@ -122,15 +173,27 @@ Result<void> RunLauncherAction(
   const std::string serialized_data = extended_action.SerializeAsString();
   CF_EXPECT(!serialized_data.empty(), "failed to serialize proto");
 
-  auto message = CF_EXPECT(LauncherActionMessage::Create(
-      LauncherAction::kExtended, extended_action_type,
-      std::move(serialized_data)));
-  CF_EXPECT(message.WriteToFd(monitor_socket));
+  const LauncherAction action = LauncherAction::kExtended;
+  CF_EXPECT(WriteAllBinaryResult(monitor_socket, &action),
+            "Error writing LauncherAction");
+  CF_EXPECT(WriteAllBinaryResult(monitor_socket, &extended_action_type),
+            "Error writing ExtendedActionType");
+  const std::uint32_t length = serialized_data.size();
+  CF_EXPECT(WriteAllBinaryResult(monitor_socket, &length),
+            "Error writing proto length");
+  if (!serialized_data.empty()) {
+    ssize_t n = WriteAll(monitor_socket, serialized_data.data(),
+                         serialized_data.size());
+    CF_EXPECTF(n > 0, "Write error: {}", monitor_socket->StrError());
+    CF_EXPECT(n == serialized_data.size(), "Unexpected EOF on write");
+  }
+
   if (timeout_seconds.has_value()) {
     CF_EXPECT(WaitForRead(monitor_socket, timeout_seconds.value()));
   }
-  LauncherResponse response =
-      CF_EXPECT(ReadFromMonitor<LauncherResponse>(monitor_socket));
+  LauncherResponse response;
+  CF_EXPECT(ReadExactBinaryResult(monitor_socket, &response),
+            "Error reading LauncherResponse");
   CF_EXPECT_EQ((int)response, (int)LauncherResponse::kSuccess);
   return {};
 }