Sway IPC: code cleanups

include/modules/sway/ipc/client.hpp

@@ -1,14 +1,10 @@
 #pragma once
 
 #include <sigc++/sigc++.h>
-#include <sys/socket.h>
-#include <sys/un.h>
-#include <unistd.h>
 
-#include <cstring>
+#include <cstdint>
-#include <memory>
+#include <functional>
 #include <mutex>
-#include <stdexcept>
 #include <string>
 
 #include "ipc.hpp"
@@ -41,8 +37,9 @@ class Ipc {
   static inline const std::string ipc_magic_ = "i3-ipc";
   static inline const size_t ipc_header_size_ = ipc_magic_.size() + 8;
 
-  const std::string getSocketPath() const;
+  static std::string getSocketPath();
-  int open(const std::string&) const;
+  static int open(const std::string&);
+
   struct ipc_response send(int fd, uint32_t type, const std::string& payload = "");
   struct ipc_response recv(int fd);
 

src/modules/sway/ipc/client.cpp

@@ -1,12 +1,22 @@
 #include "modules/sway/ipc/client.hpp"
 
-#include <cerrno>
 #include <fcntl.h>
 #include <spdlog/spdlog.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <unistd.h>
 
+#include <cerrno>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
 #include <stdexcept>
+#include <string_view>
 #include <utility>
 
+#include "modules/sway/ipc/ipc.hpp"
+
 namespace waybar::modules::sway {
 namespace {
 
@@ -30,7 +40,7 @@ void sendAll(int fd, const char* data, size_t size, const char* what) {
 }  // namespace
 
 Ipc::Ipc() {
-  const std::string& socketPath = getSocketPath();
+  const std::string socketPath = getSocketPath();
   fd_ = util::ScopedFd(open(socketPath));
   fd_event_ = util::ScopedFd(open(socketPath));
 }
@@ -51,49 +61,49 @@ Ipc::~Ipc() {
   }
 }
 
-void Ipc::setWorker(std::function<void()>&& func) { thread_ = func; }
+void Ipc::setWorker(std::function<void()>&& func) { thread_ = std::move(func); }
 
-const std::string Ipc::getSocketPath() const {
+std::string Ipc::getSocketPath() {
   const char* env = getenv("SWAYSOCK");
-  if (env != nullptr) {
+  if (env != nullptr && env[0] != '\0') {
-    return std::string(env);
+    return {env};
   }
-  std::string str;
+
-  {
+  FILE* in = popen("sway --get-socketpath 2>/dev/null", "r");
-    std::string str_buf;
+  if (in == nullptr) {
-    FILE* in;
-    char buf[512] = {0};
-    if ((in = popen("sway --get-socketpath 2>/dev/null", "r")) == nullptr) {
     throw std::runtime_error("Failed to get socket path");
   }
+
+  std::string str;
+  char buf[512] = {0};
   while (fgets(buf, sizeof(buf), in) != nullptr) {
-      str_buf.append(buf);
+    str.append(buf);
+  }
+
+  if (pclose(in) == -1) {
+    throw std::runtime_error("Failed to get socket path");
+  }
+
+  if (str.ends_with('\n')) {
+    str.pop_back();
   }
-    pclose(in);
-    str = str_buf;
   if (str.empty()) {
     throw std::runtime_error("Socket path is empty");
   }
-  }
+
-  if (str.back() == '\n') {
-    str.pop_back();
-  }
   return str;
 }
 
-int Ipc::open(const std::string& socketPath) const {
+int Ipc::open(const std::string& socketPath) {
   util::ScopedFd fd(socket(AF_UNIX, SOCK_STREAM, 0));
   if (fd == -1) {
     throw std::runtime_error("Unable to open Unix socket");
   }
   (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
-  struct sockaddr_un addr;
+  struct sockaddr_un addr{.sun_family = AF_UNIX};
-  memset(&addr, 0, sizeof(struct sockaddr_un));
-  addr.sun_family = AF_UNIX;
   strncpy(addr.sun_path, socketPath.c_str(), sizeof(addr.sun_path) - 1);
   addr.sun_path[sizeof(addr.sun_path) - 1] = 0;
-  int l = sizeof(struct sockaddr_un);
+  if (::connect(fd, reinterpret_cast<struct sockaddr*>(&addr), sizeof addr) == -1) {
-  if (::connect(fd, reinterpret_cast<struct sockaddr*>(&addr), l) == -1) {
     throw std::runtime_error("Unable to connect to Sway");
   }
   return fd.release();
@@ -102,14 +112,13 @@ int Ipc::open(const std::string& socketPath) const {
 struct Ipc::ipc_response Ipc::recv(int fd) {
   std::string header;
   header.resize(ipc_header_size_);
-  auto data32 = reinterpret_cast<uint32_t*>(header.data() + ipc_magic_.size());
-  size_t total = 0;
 
+  size_t total = 0;
   while (total < ipc_header_size_) {
-    auto res = ::recv(fd, header.data() + total, ipc_header_size_ - total, 0);
+    const ssize_t res = ::recv(fd, header.data() + total, ipc_header_size_ - total, 0);
     if (fd_event_ == -1 || fd_ == -1) {
       // IPC is closed so just return an empty response
-      return {0, 0, ""};
+      return {.size = 0, .type = 0, .payload = ""};
     }
     if (res < 0) {
       if (errno == EINTR || errno == EAGAIN) {
@@ -122,16 +131,18 @@ struct Ipc::ipc_response Ipc::recv(int fd) {
     }
     total += static_cast<size_t>(res);
   }
-  auto magic = std::string(header.data(), header.data() + ipc_magic_.size());
+  if (std::string_view(header.data(), ipc_magic_.size()) != ipc_magic_) {
-  if (magic != ipc_magic_) {
     throw std::runtime_error("Invalid IPC magic");
   }
 
-  total = 0;
+  const auto* data32 = reinterpret_cast<uint32_t*>(header.data() + ipc_magic_.size());
+
   std::string payload;
   payload.resize(data32[0]);
+
+  total = 0;
   while (total < data32[0]) {
-    auto res = ::recv(fd, payload.data() + total, data32[0] - total, 0);
+    const ssize_t res = ::recv(fd, payload.data() + total, data32[0] - total, 0);
     if (res < 0) {
       if (errno == EINTR || errno == EAGAIN) {
         continue;
@@ -143,19 +154,21 @@ struct Ipc::ipc_response Ipc::recv(int fd) {
     }
     total += static_cast<size_t>(res);
   }
-  return {data32[0], data32[1], std::move(payload)};
+
+  return {.size = data32[0], .type = data32[1], .payload = std::move(payload)};
 }
 
 struct Ipc::ipc_response Ipc::send(int fd, uint32_t type, const std::string& payload) {
   std::string header;
   header.resize(ipc_header_size_);
-  auto data32 = reinterpret_cast<uint32_t*>(header.data() + ipc_magic_.size());
+  memcpy(header.data(), ipc_magic_.data(), ipc_magic_.size());
-  memcpy(header.data(), ipc_magic_.c_str(), ipc_magic_.size());
+  auto* data32 = reinterpret_cast<uint32_t*>(header.data() + ipc_magic_.size());
   data32[0] = payload.size();
   data32[1] = type;
 
   sendAll(fd, header.data(), ipc_header_size_, "Unable to send IPC header");
   sendAll(fd, payload.data(), payload.size(), "Unable to send IPC payload");
+
   return Ipc::recv(fd);
 }