crashReporter: cleanup code (#12534)

various code cleanups, reorders, move off of global NS

src/Compositor.cpp

@@ -27,7 +27,7 @@
 #include <filesystem>
 #include <unordered_set>
 #include "debug/HyprCtl.hpp"
-#include "debug/CrashReporter.hpp"
+#include "debug/crash/CrashReporter.hpp"
 #ifdef USES_SYSTEMD
 #include <helpers/SdDaemon.hpp> // for SdNotify
 #endif
@@ -113,7 +113,7 @@ static void handleUnrecoverableSignal(int sig) {
     });
     alarm(15);
 
-    NCrashReporter::createAndSaveCrash(sig);
+    CrashReporter::createAndSaveCrash(sig);
 
     abort();
 }

src/debug/crash/CrashReporter.cpp

@@ -6,36 +6,43 @@
 #include <cerrno>
 #include <sys/stat.h>
 #include <filesystem>
-#include "../helpers/MiscFunctions.hpp"
+#include "../../helpers/MiscFunctions.hpp"
 
-#include "../plugins/PluginSystem.hpp"
+#include "../../plugins/PluginSystem.hpp"
-#include "../signal-safe.hpp"
+#include "SignalSafe.hpp"
 
 #if defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__)
 #include <sys/sysctl.h>
 #endif
 
-static char const* const MESSAGES[] = {"Sorry, didn't mean to...",
+static char const* const MESSAGES[] = {
-                                       "This was an accident, I swear!",
+    "Sorry, didn't mean to...",
-                                       "Calm down, it was a misinput! MISINPUT!",
+    "This was an accident, I swear!",
-                                       "Oops",
+    "Calm down, it was a misinput! MISINPUT!",
-                                       "Vaxry is going to be upset.",
+    "Oops",
-                                       "Who tried dividing by zero?!",
+    "Vaxry is going to be upset.",
-                                       "Maybe you should try dusting your PC in the meantime?",
+    "Who tried dividing by zero?!",
-                                       "I tried so hard, and got so far...",
+    "Maybe you should try dusting your PC in the meantime?",
-                                       "I don't feel so good...",
+    "I tried so hard, and got so far...",
-                                       "*thud*",
+    "I don't feel so good...",
-                                       "Well this is awkward.",
+    "*thud*",
-                                       "\"stable\"",
+    "Well this is awkward.",
-                                       "I hope you didn't have any unsaved progress.",
+    "\"stable\"",
-                                       "All these computers..."};
+    "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?",
+};
 
 // <random> 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                         HOME       = SignalSafe::getenv("HOME");
-        const auto             CACHE_HOME = sigGetenv("XDG_CACHE_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);
+            SignalSafe::CBufFileWriter<64> stderrOut(STDERR_FILENO);
-            stderr_out += "Hyprland has crashed :( Consult the crash report at ";
+            stderrOut += "Hyprland has crashed :( Consult the crash report at ";
-            if (!reportPath.boundsExceeded()) {
+            if (!reportPath.boundsExceeded())
-                stderr_out += reportPath.getStr();
+                stderrOut += reportPath.getStr();
-            } else {
+            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.]";
+                stderrOut += "[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";
+            stderrOut += " for more information.\n";
-            stderr_out.flush();
+            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: ";

src/debug/crash/CrashReporter.hpp

@@ -1,7 +1,5 @@
 #pragma once
 
-#include "../defines.hpp"
+namespace CrashReporter {
-
-namespace NCrashReporter {
     void createAndSaveCrash(int sig);
 };

src/debug/crash/SignalSafe.cpp

@@ -1,4 +1,4 @@
-#include "signal-safe.hpp"
+#include "SignalSafe.hpp"
 
 #ifndef __GLIBC__
 #include <signal.h>
@@ -7,11 +7,13 @@
 #include <unistd.h>
 #include <cstring>
 
+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__)

src/debug/crash/SignalSafe.hpp

@@ -0,0 +1,203 @@
+#pragma once
+
+#include "defines.hpp"
+#include <cstring>
+
+namespace SignalSafe {
+    template <uint16_t N>
+    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 <uint16_t BUFSIZE>
+    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<size_t>(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 += "<pipe(pipefd) failed with";
+                writeNum(errno);
+                *this += ">\n";
+                return;
+            }
+
+            // terminate child instead of waiting
+            {
+                struct sigaction act;
+                act.sa_handler = SIG_DFL;
+                sigemptyset(&act.sa_mask);
+                act.sa_flags = SA_NOCLDWAIT;
+#ifdef SA_RESTORER
+                act.sa_restorer = NULL;
+#endif
+                sigaction(SIGCHLD, &act, nullptr);
+            }
+
+            const pid_t pid = fork();
+
+            if (pid < 0) {
+                *this += "<fork() failed with ";
+                writeNum(errno);
+                *this += ">\n";
+                return;
+            }
+
+            if (pid == 0) {
+                close(pipefd[0]);
+                dup2(pipefd[1], STDOUT_FILENO);
+                char const* const argv[] = {"/bin/sh", "-c", cmd, nullptr};
+                execv("/bin/sh", cc<char* const*>(argv));
+
+                CBufFileWriter<64> failmsg(pipefd[1]);
+                failmsg += "<execv(";
+                failmsg += cmd;
+                failmsg += ") resulted in errno ";
+                failmsg.write(errno);
+                failmsg += ">\n";
+                close(pipefd[1]);
+                abort();
+            } else {
+                close(pipefd[1]);
+                int64_t len = 0;
+                char    readbuf[256];
+                while ((len = read(pipefd[0], readbuf, 256)) > 0) {
+                    write(readbuf, len);
+                }
+                if (len < 0) {
+                    *this += "<interrupted, read() resulted in errno ";
+                    writeNum(errno);
+                    *this += ">\n";
+                }
+                close(pipefd[0]);
+            }
+        }
+
+        void flush() {
+            size_t i = 0;
+            while (i < m_writeBufPos) {
+                auto written = ::write(m_fd, m_writeBuf + i, m_writeBufPos - i);
+                if (written <= 0) {
+                    return;
+                }
+                i += written;
+            }
+            m_writeBufPos = 0;
+        }
+
+      private:
+        char   m_writeBuf[BUFSIZE] = {0};
+        size_t m_writeBufPos       = 0;
+        int    m_fd                = 0;
+    };
+
+    char const* getenv(const char* name);
+    char const* strsignal(int sig);
+}

src/signal-safe.hpp

@@ -1,175 +0,0 @@
-#pragma once
-
-#include "defines.hpp"
-#include <cstring>
-
-template <uint16_t N>
-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 <uint16_t BUFSIZE>
-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<size_t>(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 += "<pipe(pipefd) failed with";
-            writeNum(errno);
-            *this += ">\n";
-            return;
-        }
-        // terminate child instead of waiting
-        {
-            struct sigaction act;
-            act.sa_handler = SIG_DFL;
-            sigemptyset(&act.sa_mask);
-            act.sa_flags = SA_NOCLDWAIT;
-#ifdef SA_RESTORER
-            act.sa_restorer = NULL;
-#endif
-            sigaction(SIGCHLD, &act, nullptr);
-        }
-        const pid_t pid = fork();
-        if (pid < 0) {
-            *this += "<fork() failed with ";
-            writeNum(errno);
-            *this += ">\n";
-            return;
-        }
-        if (pid == 0) {
-            close(pipefd[0]);
-            dup2(pipefd[1], STDOUT_FILENO);
-            char const* const argv[] = {"/bin/sh", "-c", cmd, nullptr};
-            execv("/bin/sh", const_cast<char* const*>(argv));
-
-            CBufFileWriter<64> failmsg(pipefd[1]);
-            failmsg += "<execv(";
-            failmsg += cmd;
-            failmsg += ") resulted in errno ";
-            failmsg.write(errno);
-            failmsg += ">\n";
-            close(pipefd[1]);
-            abort();
-        } else {
-            close(pipefd[1]);
-            int64_t len = 0;
-            char    readbuf[256];
-            while ((len = read(pipefd[0], readbuf, 256)) > 0) {
-                write(readbuf, len);
-            }
-            if (len < 0) {
-                *this += "<interrupted, read() resulted in errno ";
-                writeNum(errno);
-                *this += ">\n";
-            }
-            close(pipefd[0]);
-        }
-    }
-    void flush() {
-        size_t i = 0;
-        while (i < m_writeBufPos) {
-            auto written = ::write(m_fd, m_writeBuf + i, m_writeBufPos - i);
-            if (written <= 0) {
-                return;
-            }
-            i += written;
-        }
-        m_writeBufPos = 0;
-    }
-
-  private:
-    char   m_writeBuf[BUFSIZE] = {0};
-    size_t m_writeBufPos       = 0;
-    int    m_fd                = 0;
-};
-
-char const* sigGetenv(char const* name);
-
-char const* sigStrsignal(int sig);