From 93e5e92b0ae7809ee0f64d94d51e210c476ee823 Mon Sep 17 00:00:00 2001 From: Vaxry <43317083+vaxerski@users.noreply.github.com> Date: Wed, 3 Dec 2025 16:01:45 +0000 Subject: [PATCH] crashReporter: cleanup code (#12534) various code cleanups, reorders, move off of global NS --- src/Compositor.cpp | 4 +- src/debug/{ => crash}/CrashReporter.cpp | 83 +++---- src/debug/{ => crash}/CrashReporter.hpp | 4 +- .../crash/SignalSafe.cpp} | 8 +- src/debug/crash/SignalSafe.hpp | 203 ++++++++++++++++++ src/signal-safe.hpp | 175 --------------- 6 files changed, 255 insertions(+), 222 deletions(-) rename src/debug/{ => crash}/CrashReporter.cpp (76%) rename src/debug/{ => crash}/CrashReporter.hpp (50%) rename src/{signal-safe.cpp => debug/crash/SignalSafe.cpp} (78%) create mode 100644 src/debug/crash/SignalSafe.hpp delete mode 100644 src/signal-safe.hpp diff --git a/src/Compositor.cpp b/src/Compositor.cpp index 643aad1b..9c812b37 100644 --- a/src/Compositor.cpp +++ b/src/Compositor.cpp @@ -27,7 +27,7 @@ #include #include #include "debug/HyprCtl.hpp" -#include "debug/CrashReporter.hpp" +#include "debug/crash/CrashReporter.hpp" #ifdef USES_SYSTEMD #include // for SdNotify #endif @@ -113,7 +113,7 @@ static void handleUnrecoverableSignal(int sig) { }); alarm(15); - NCrashReporter::createAndSaveCrash(sig); + CrashReporter::createAndSaveCrash(sig); abort(); } diff --git a/src/debug/CrashReporter.cpp b/src/debug/crash/CrashReporter.cpp similarity index 76% rename from src/debug/CrashReporter.cpp rename to src/debug/crash/CrashReporter.cpp index 9e871903..1b18fce4 100644 --- a/src/debug/CrashReporter.cpp +++ b/src/debug/crash/CrashReporter.cpp @@ -6,36 +6,43 @@ #include #include #include -#include "../helpers/MiscFunctions.hpp" +#include "../../helpers/MiscFunctions.hpp" -#include "../plugins/PluginSystem.hpp" -#include "../signal-safe.hpp" +#include "../../plugins/PluginSystem.hpp" +#include "SignalSafe.hpp" #if defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) #include #endif -static char const* const MESSAGES[] = {"Sorry, didn't mean to...", - "This was an accident, I swear!", - "Calm down, it was a misinput! MISINPUT!", - "Oops", - "Vaxry is going to be upset.", - "Who tried dividing by zero?!", - "Maybe you should try dusting your PC in the meantime?", - "I tried so hard, and got so far...", - "I don't feel so good...", - "*thud*", - "Well this is awkward.", - "\"stable\"", - "I hope you didn't have any unsaved progress.", - "All these computers..."}; +static char const* const MESSAGES[] = { + "Sorry, didn't mean to...", + "This was an accident, I swear!", + "Calm down, it was a misinput! MISINPUT!", + "Oops", + "Vaxry is going to be upset.", + "Who tried dividing by zero?!", + "Maybe you should try dusting your PC in the meantime?", + "I tried so hard, and got so far...", + "I don't feel so good...", + "*thud*", + "Well this is awkward.", + "\"stable\"", + "I hope you didn't have any unsaved progress.", + "All these computers...", + "The math isn't mathing...", + "We've got an imposter in the code!", + "Well, at least the crash reporter didn't crash!", + "Everything's just fi-", + "Have you tried asking Hyprland politely not to crash?", +}; // is not async-signal-safe, fake it with time(NULL) instead -char const* getRandomMessage() { +static char const* getRandomMessage() { return MESSAGES[time(nullptr) % (sizeof(MESSAGES) / sizeof(MESSAGES[0]))]; } -[[noreturn]] inline void exitWithError(char const* err) { +[[noreturn]] static inline void exitWithError(char const* err) { write(STDERR_FILENO, err, strlen(err)); // perror() is not signal-safe, but we use it here // because if the crash-handler already crashed, it can't get any worse. @@ -43,17 +50,17 @@ char const* getRandomMessage() { abort(); } -void NCrashReporter::createAndSaveCrash(int sig) { +void CrashReporter::createAndSaveCrash(int sig) { int reportFd = -1; // We're in the signal handler, so we *only* have stack memory. // To save as much stack memory as possible, // destroy things as soon as possible. { - CMaxLengthCString<255> reportPath; + SignalSafe::CMaxLengthCString<255> reportPath; - const auto HOME = sigGetenv("HOME"); - const auto CACHE_HOME = sigGetenv("XDG_CACHE_HOME"); + const auto HOME = SignalSafe::getenv("HOME"); + const auto CACHE_HOME = SignalSafe::getenv("XDG_CACHE_HOME"); if (CACHE_HOME && CACHE_HOME[0] != '\0') { reportPath += CACHE_HOME; @@ -67,32 +74,30 @@ void NCrashReporter::createAndSaveCrash(int sig) { } int ret = mkdir(reportPath.getStr(), S_IRWXU); - //__asm__("int $3"); - if (ret < 0 && errno != EEXIST) { + if (ret < 0 && errno != EEXIST) exitWithError("failed to mkdir() crash report directory\n"); - } + reportPath += "/hyprlandCrashReport"; reportPath.writeNum(getpid()); reportPath += ".txt"; { - CBufFileWriter<64> stderr_out(STDERR_FILENO); - stderr_out += "Hyprland has crashed :( Consult the crash report at "; - if (!reportPath.boundsExceeded()) { - stderr_out += reportPath.getStr(); - } else { - stderr_out += "[ERROR: Crash report path does not fit into memory! Check if your $CACHE_HOME/$HOME is too deeply nested. Max 255 characters.]"; - } - stderr_out += " for more information.\n"; - stderr_out.flush(); + SignalSafe::CBufFileWriter<64> stderrOut(STDERR_FILENO); + stderrOut += "Hyprland has crashed :( Consult the crash report at "; + if (!reportPath.boundsExceeded()) + stderrOut += reportPath.getStr(); + else + stderrOut += "[ERROR: Crash report path does not fit into memory! Check if your $CACHE_HOME/$HOME is too deeply nested. Max 255 characters.]"; + + stderrOut += " for more information.\n"; + stderrOut.flush(); } reportFd = open(reportPath.getStr(), O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR); - if (reportFd < 0) { + if (reportFd < 0) exitWithError("Failed to open crash report path for writing"); - } } - CBufFileWriter<512> finalCrashReport(reportFd); + SignalSafe::CBufFileWriter<512> finalCrashReport(reportFd); finalCrashReport += "--------------------------------------------\n Hyprland Crash Report\n--------------------------------------------\n"; finalCrashReport += getRandomMessage(); @@ -101,7 +106,7 @@ void NCrashReporter::createAndSaveCrash(int sig) { finalCrashReport += "Hyprland received signal "; finalCrashReport.writeNum(sig); finalCrashReport += '('; - finalCrashReport += sigStrsignal(sig); + finalCrashReport += SignalSafe::strsignal(sig); finalCrashReport += ")\nVersion: "; finalCrashReport += GIT_COMMIT_HASH; finalCrashReport += "\nTag: "; diff --git a/src/debug/CrashReporter.hpp b/src/debug/crash/CrashReporter.hpp similarity index 50% rename from src/debug/CrashReporter.hpp rename to src/debug/crash/CrashReporter.hpp index 0ba48e7c..661f702f 100644 --- a/src/debug/CrashReporter.hpp +++ b/src/debug/crash/CrashReporter.hpp @@ -1,7 +1,5 @@ #pragma once -#include "../defines.hpp" - -namespace NCrashReporter { +namespace CrashReporter { void createAndSaveCrash(int sig); }; \ No newline at end of file diff --git a/src/signal-safe.cpp b/src/debug/crash/SignalSafe.cpp similarity index 78% rename from src/signal-safe.cpp rename to src/debug/crash/SignalSafe.cpp index baee7b44..22717f1b 100644 --- a/src/signal-safe.cpp +++ b/src/debug/crash/SignalSafe.cpp @@ -1,4 +1,4 @@ -#include "signal-safe.hpp" +#include "SignalSafe.hpp" #ifndef __GLIBC__ #include @@ -7,11 +7,13 @@ #include #include +using namespace SignalSafe; + // NOLINTNEXTLINE extern "C" char** environ; // -char const* sigGetenv(char const* name) { +char const* SignalSafe::getenv(char const* name) { const size_t len = strlen(name); for (char** var = environ; *var != nullptr; var++) { if (strncmp(*var, name, len) == 0 && (*var)[len] == '=') { @@ -21,7 +23,7 @@ char const* sigGetenv(char const* name) { return nullptr; } -char const* sigStrsignal(int sig) { +char const* SignalSafe::strsignal(int sig) { #ifdef __GLIBC__ return sigabbrev_np(sig); #elif defined(__DragonFly__) || defined(__FreeBSD__) diff --git a/src/debug/crash/SignalSafe.hpp b/src/debug/crash/SignalSafe.hpp new file mode 100644 index 00000000..8ec967fe --- /dev/null +++ b/src/debug/crash/SignalSafe.hpp @@ -0,0 +1,203 @@ +#pragma once + +#include "defines.hpp" +#include + +namespace SignalSafe { + template + class CMaxLengthCString { + public: + CMaxLengthCString() { + m_str[0] = '\0'; + } + + void operator+=(char const* rhs) { + write(rhs, strlen(rhs)); + } + + void write(char const* data, size_t len) { + if (m_boundsExceeded || m_strPos + len >= N) { + m_boundsExceeded = true; + return; + } + memcpy(m_str + m_strPos, data, len); + m_strPos += len; + m_str[m_strPos] = '\0'; + } + + void write(char c) { + if (m_boundsExceeded || m_strPos + 1 >= N) { + m_boundsExceeded = true; + return; + } + m_str[m_strPos] = c; + m_strPos++; + } + + void writeNum(size_t num) { + size_t d = 1; + + while (num / 10 >= d) { + d *= 10; + } + + while (num > 0) { + char c = '0' + (num / d); + write(c); + num %= d; + d /= 10; + } + } + + char const* getStr() { + return m_str; + } + + bool boundsExceeded() { + return m_boundsExceeded; + } + + private: + char m_str[N]; + size_t m_strPos = 0; + bool m_boundsExceeded = false; + }; + + template + class CBufFileWriter { + public: + CBufFileWriter(int fd_) : m_fd(fd_) { + ; + } + + ~CBufFileWriter() { + flush(); + } + + void write(char const* data, size_t len) { + while (len > 0) { + size_t to_add = std::min(len, sc(BUFSIZE) - m_writeBufPos); + memcpy(m_writeBuf + m_writeBufPos, data, to_add); + data += to_add; + len -= to_add; + m_writeBufPos += to_add; + if (m_writeBufPos == BUFSIZE) + flush(); + } + } + + void write(char c) { + if (m_writeBufPos == BUFSIZE) + flush(); + m_writeBuf[m_writeBufPos] = c; + m_writeBufPos++; + } + + void operator+=(char const* str) { + write(str, strlen(str)); + } + + void operator+=(std::string_view str) { + write(str.data(), str.size()); + } + + void operator+=(char c) { + write(c); + } + + void writeNum(size_t num) { + size_t d = 1; + + while (num / 10 >= d) { + d *= 10; + } + + while (num > 0) { + char c = '0' + (num / d); + write(c); + num %= d; + d /= 10; + } + } + + void writeCmdOutput(const char* cmd) { + int pipefd[2]; + if (pipe(pipefd) < 0) { + *this += "(argv)); + + CBufFileWriter<64> failmsg(pipefd[1]); + failmsg += " 0) { + write(readbuf, len); + } + if (len < 0) { + *this += " - -template -class CMaxLengthCString { - public: - CMaxLengthCString() { - m_str[0] = '\0'; - } - void operator+=(char const* rhs) { - write(rhs, strlen(rhs)); - } - void write(char const* data, size_t len) { - if (m_boundsExceeded || m_strPos + len >= N) { - m_boundsExceeded = true; - return; - } - memcpy(m_str + m_strPos, data, len); - m_strPos += len; - m_str[m_strPos] = '\0'; - } - void write(char c) { - if (m_boundsExceeded || m_strPos + 1 >= N) { - m_boundsExceeded = true; - return; - } - m_str[m_strPos] = c; - m_strPos++; - } - void writeNum(size_t num) { - size_t d = 1; - while (num / 10 >= d) - d *= 10; - while (num > 0) { - char c = '0' + (num / d); - write(c); - num %= d; - d /= 10; - } - } - char const* getStr() { - return m_str; - }; - bool boundsExceeded() { - return m_boundsExceeded; - }; - - private: - char m_str[N]; - size_t m_strPos = 0; - bool m_boundsExceeded = false; -}; - -template -class CBufFileWriter { - public: - CBufFileWriter(int fd_) : m_fd(fd_) {} - ~CBufFileWriter() { - flush(); - } - void write(char const* data, size_t len) { - while (len > 0) { - size_t to_add = std::min(len, sc(BUFSIZE) - m_writeBufPos); - memcpy(m_writeBuf + m_writeBufPos, data, to_add); - data += to_add; - len -= to_add; - m_writeBufPos += to_add; - if (m_writeBufPos == BUFSIZE) - flush(); - } - } - void write(char c) { - if (m_writeBufPos == BUFSIZE) - flush(); - m_writeBuf[m_writeBufPos] = c; - m_writeBufPos++; - } - void operator+=(char const* str) { - write(str, strlen(str)); - } - void operator+=(std::string_view str) { - write(str.data(), str.size()); - } - void operator+=(char c) { - write(c); - } - void writeNum(size_t num) { - size_t d = 1; - while (num / 10 >= d) - d *= 10; - while (num > 0) { - char c = '0' + (num / d); - write(c); - num %= d; - d /= 10; - } - } - void writeCmdOutput(const char* cmd) { - int pipefd[2]; - if (pipe(pipefd) < 0) { - *this += "(argv)); - - CBufFileWriter<64> failmsg(pipefd[1]); - failmsg += " 0) { - write(readbuf, len); - } - if (len < 0) { - *this += "