From bdd7271da9aa30aecde9ef703a38a9195ec284c5 Mon Sep 17 00:00:00 2001 From: yangyingchao Date: Sat, 6 Jan 2024 12:57:27 +0800 Subject: [PATCH] Improvements for Hyprland backend 1. Fix warnings reported by clang tidy 2. Use unique lock instead of manully lock/unlock on mutex. The RAII style locking makes sure mutex is unlocked when exceptions are thrown --- include/modules/hyprland/backend.hpp | 10 ++-- src/modules/hyprland/backend.cpp | 79 +++++++++++----------------- 2 files changed, 35 insertions(+), 54 deletions(-) diff --git a/include/modules/hyprland/backend.hpp b/include/modules/hyprland/backend.hpp index 7d97b553..d197df3a 100644 --- a/include/modules/hyprland/backend.hpp +++ b/include/modules/hyprland/backend.hpp @@ -1,11 +1,9 @@ #pragma once -#include #include #include #include #include -#include #include #include "util/json.hpp" @@ -25,16 +23,16 @@ class IPC { void registerForIPC(const std::string&, EventHandler*); void unregisterForIPC(EventHandler*); - std::string getSocket1Reply(const std::string& rq); + static std::string getSocket1Reply(const std::string& rq); Json::Value getSocket1JsonReply(const std::string& rq); private: void startIPC(); void parseIPC(const std::string&); - std::mutex callbackMutex; - util::JsonParser parser_; - std::list> callbacks; + std::mutex m_callbackMutex; + util::JsonParser m_parser; + std::list> m_callbacks; }; inline std::unique_ptr gIPC; diff --git a/src/modules/hyprland/backend.cpp b/src/modules/hyprland/backend.cpp index 5a48d369..3c8313fc 100644 --- a/src/modules/hyprland/backend.cpp +++ b/src/modules/hyprland/backend.cpp @@ -1,21 +1,16 @@ #include "modules/hyprland/backend.hpp" -#include #include #include #include -#include -#include -#include #include #include #include #include #include -#include -#include #include +#include namespace waybar::modules::hyprland { @@ -24,9 +19,9 @@ void IPC::startIPC() { std::thread([&]() { // check for hyprland - const char* HIS = getenv("HYPRLAND_INSTANCE_SIGNATURE"); + const char* his = getenv("HYPRLAND_INSTANCE_SIGNATURE"); - if (!HIS) { + if (his == nullptr) { spdlog::warn("Hyprland is not running, Hyprland IPC will not be available."); return; } @@ -45,8 +40,8 @@ void IPC::startIPC() { addr.sun_family = AF_UNIX; - // socket path - std::string socketPath = "/tmp/hypr/" + std::string(HIS) + "/.socket2.sock"; + // socket path, specified by EventManager of Hyprland + std::string socketPath = "/tmp/hypr/" + std::string(his) + "/.socket2.sock"; strncpy(addr.sun_path, socketPath.c_str(), sizeof(addr.sun_path) - 1); @@ -61,10 +56,9 @@ void IPC::startIPC() { auto file = fdopen(socketfd, "r"); - while (1) { - // read - + while (true) { char buffer[1024]; // Hyprland socket2 events are max 1024 bytes + auto recievedCharPtr = fgets(buffer, 1024, file); if (!recievedCharPtr) { @@ -72,28 +66,21 @@ void IPC::startIPC() { continue; } - callbackMutex.lock(); - std::string messageRecieved(buffer); - messageRecieved = messageRecieved.substr(0, messageRecieved.find_first_of('\n')); - spdlog::debug("hyprland IPC received {}", messageRecieved); - parseIPC(messageRecieved); - callbackMutex.unlock(); - std::this_thread::sleep_for(std::chrono::milliseconds(1)); } }).detach(); } void IPC::parseIPC(const std::string& ev) { - // todo std::string request = ev.substr(0, ev.find_first_of('>')); + std::unique_lock lock(m_callbackMutex); - for (auto& [eventname, handler] : callbacks) { + for (auto& [eventname, handler] : m_callbacks) { if (eventname == request) { handler->onEvent(ev); } @@ -104,11 +91,9 @@ void IPC::registerForIPC(const std::string& ev, EventHandler* ev_handler) { if (!ev_handler) { return; } - callbackMutex.lock(); - callbacks.emplace_back(std::make_pair(ev, ev_handler)); - - callbackMutex.unlock(); + std::unique_lock lock(m_callbackMutex); + m_callbacks.emplace_back(ev, ev_handler); } void IPC::unregisterForIPC(EventHandler* ev_handler) { @@ -116,37 +101,35 @@ void IPC::unregisterForIPC(EventHandler* ev_handler) { return; } - callbackMutex.lock(); + std::unique_lock lock(m_callbackMutex); - for (auto it = callbacks.begin(); it != callbacks.end();) { - auto it_current = it; - it++; - auto& [eventname, handler] = *it_current; + for (auto it = m_callbacks.begin(); it != m_callbacks.end();) { + auto& [eventname, handler] = *it; if (handler == ev_handler) { - callbacks.erase(it_current); + m_callbacks.erase(it++); + } else { + ++it; } } - - callbackMutex.unlock(); } std::string IPC::getSocket1Reply(const std::string& rq) { // basically hyprctl - struct addrinfo ai_hints; - struct addrinfo* ai_res = NULL; - const auto SERVERSOCKET = socket(AF_UNIX, SOCK_STREAM, 0); + struct addrinfo aiHints; + struct addrinfo* aiRes = nullptr; + const auto serverSocket = socket(AF_UNIX, SOCK_STREAM, 0); - if (SERVERSOCKET < 0) { + if (serverSocket < 0) { spdlog::error("Hyprland IPC: Couldn't open a socket (1)"); return ""; } - memset(&ai_hints, 0, sizeof(struct addrinfo)); - ai_hints.ai_family = AF_UNSPEC; - ai_hints.ai_socktype = SOCK_STREAM; + memset(&aiHints, 0, sizeof(struct addrinfo)); + aiHints.ai_family = AF_UNSPEC; + aiHints.ai_socktype = SOCK_STREAM; - if (getaddrinfo("localhost", NULL, &ai_hints, &ai_res) != 0) { + if (getaddrinfo("localhost", nullptr, &aiHints, &aiRes) != 0) { spdlog::error("Hyprland IPC: Couldn't get host (2)"); return ""; } @@ -173,13 +156,13 @@ std::string IPC::getSocket1Reply(const std::string& rq) { return ""; } - if (connect(SERVERSOCKET, reinterpret_cast(&serverAddress), sizeof(serverAddress)) < + if (connect(serverSocket, reinterpret_cast(&serverAddress), sizeof(serverAddress)) < 0) { spdlog::error("Hyprland IPC: Couldn't connect to " + socketPath + ". (3)"); return ""; } - auto sizeWritten = write(SERVERSOCKET, rq.c_str(), rq.length()); + auto sizeWritten = write(serverSocket, rq.c_str(), rq.length()); if (sizeWritten < 0) { spdlog::error("Hyprland IPC: Couldn't write (4)"); @@ -190,22 +173,22 @@ std::string IPC::getSocket1Reply(const std::string& rq) { std::string response; do { - sizeWritten = read(SERVERSOCKET, buffer, 8192); + sizeWritten = read(serverSocket, buffer, 8192); if (sizeWritten < 0) { spdlog::error("Hyprland IPC: Couldn't read (5)"); - close(SERVERSOCKET); + close(serverSocket); return ""; } response.append(buffer, sizeWritten); } while (sizeWritten > 0); - close(SERVERSOCKET); + close(serverSocket); return response; } Json::Value IPC::getSocket1JsonReply(const std::string& rq) { - return parser_.parse(getSocket1Reply("j/" + rq)); + return m_parser.parse(getSocket1Reply("j/" + rq)); } } // namespace waybar::modules::hyprland