Merge branch 'signal_safe' of cgzones/htop

3 files changed, 87 insertions, 47 deletions

diff --git a/CRT.c b/CRT.c
index 3c6c8c49..149cd519 100644
--- a/CRT.c
+++ b/CRT.c
@@ -843,9 +843,11 @@ static void CRT_handleSIGTERM(int sgn) {
    if (!signal_str)
       signal_str = "unknown reason";
 
-   fprintf(stderr,
+   char err_buf[512];
+   snprintf(err_buf, sizeof(err_buf),
            "A signal %d (%s) was received, exiting without persisting settings to htoprc.\n",
            sgn, signal_str);
+   full_write_str(STDERR_FILENO, err_buf);
    _exit(0);
 }
 
@@ -912,7 +914,7 @@ static void dumpStderr(void) {
 
       if (res > 0) {
          if (!header) {
-            fprintf(stderr, ">>>>>>>>>> stderr output >>>>>>>>>>\n");
+            full_write_str(STDERR_FILENO, ">>>>>>>>>> stderr output >>>>>>>>>>\n");
             header = true;
          }
          full_write(STDERR_FILENO, buffer, res);
@@ -920,7 +922,7 @@ static void dumpStderr(void) {
    }
 
    if (header)
-      fprintf(stderr, "\n<<<<<<<<<< stderr output <<<<<<<<<<\n");
+      full_write_str(STDERR_FILENO, "\n<<<<<<<<<< stderr output <<<<<<<<<<\n");
 
    close(stderrRedirectNewFd);
    stderrRedirectNewFd = -1;
@@ -1185,6 +1187,8 @@ static void print_backtrace(void) {
 
    unsigned int item = 0;
 
+   char err_buf[1024];
+
    while (unw_step(&cursor) > 0) {
       unw_word_t pc;
       unw_get_reg(&cursor, UNW_REG_IP, &pc);
@@ -1213,7 +1217,8 @@ static void print_backtrace(void) {
       const bool is_signal_frame = unw_is_signal_frame(&cursor) > 0;
       const char* frame = is_signal_frame ? "  {signal frame}" : "";
 
-      fprintf(stderr, "%2u: %#14lx  %s  (%s+%#lx)  [%p]%s\n", item++, pc, fname, symbolName, offset, ptr, frame);
+      snprintf(err_buf, sizeof(err_buf), "%2u: %#14lx  %s  (%s+%#lx)  [%p]%s\n", item++, pc, fname, symbolName, offset, ptr, frame);
+      full_write_str(STDERR_FILENO, err_buf);
    }
 #elif defined(HAVE_EXECINFO_H)
    void* backtraceArray[256];
@@ -1229,7 +1234,9 @@ static void print_backtrace(void) {
 void CRT_handleSIGSEGV(int signal) {
    CRT_done();
 
-   fprintf(stderr, "\n\n"
+   char err_buf[512];
+
+   snprintf(err_buf, sizeof(err_buf), "\n\n"
       "FATAL PROGRAM ERROR DETECTED\n"
       "============================\n"
       "Please check at https://htop.dev/issues whether this issue has already been reported.\n"
@@ -1240,12 +1247,13 @@ void CRT_handleSIGSEGV(int signal) {
       "  - Likely steps to reproduce (How did it happen?)\n",
       program
    );
+   full_write_str(STDERR_FILENO, err_buf);
 
 #ifdef PRINT_BACKTRACE
-   fprintf(stderr, "  - Backtrace of the issue (see below)\n");
+   full_write_str(STDERR_FILENO, "  - Backtrace of the issue (see below)\n");
 #endif
 
-   fprintf(stderr,
+   full_write_str(STDERR_FILENO,
       "\n"
    );
 
@@ -1253,29 +1261,30 @@ void CRT_handleSIGSEGV(int signal) {
    if (!signal_str) {
       signal_str = "unknown reason";
    }
-   fprintf(stderr,
+   snprintf(err_buf, sizeof(err_buf),
       "Error information:\n"
       "------------------\n"
       "A signal %d (%s) was received.\n"
       "\n",
       signal, signal_str
    );
+   full_write_str(STDERR_FILENO, err_buf);
 
-   fprintf(stderr,
+   full_write_str(STDERR_FILENO,
       "Setting information:\n"
       "--------------------\n");
    Settings_write(CRT_crashSettings, true);
-   fprintf(stderr, "\n\n");
+   full_write_str(STDERR_FILENO, "\n\n");
 
 #ifdef PRINT_BACKTRACE
-   fprintf(stderr,
+   full_write_str(STDERR_FILENO,
       "Backtrace information:\n"
       "----------------------\n"
    );
 
    print_backtrace();
 
-   fprintf(stderr,
+   snprintf(err_buf, sizeof(err_buf),
       "\n"
       "To make the above information more practical to work with, "
       "please also provide a disassembly of your %s binary. "
@@ -1283,31 +1292,34 @@ void CRT_handleSIGSEGV(int signal) {
       "\n",
       program
    );
+   full_write_str(STDERR_FILENO, err_buf);
 
 #ifdef HTOP_DARWIN
-   fprintf(stderr, "   otool -tvV `which %s` > ~/%s.otool\n", program, program);
+   snprintf(err_buf, sizeof(err_buf), "   otool -tvV `which %s` > ~/%s.otool\n", program, program);
 #else
-   fprintf(stderr, "   objdump -d -S -w `which %s` > ~/%s.objdump\n", program, program);
+   snprintf(err_buf, sizeof(err_buf), "   objdump -d -S -w `which %s` > ~/%s.objdump\n", program, program);
 #endif
+   full_write_str(STDERR_FILENO, err_buf);
 
-   fprintf(stderr,
+   full_write_str(STDERR_FILENO,
       "\n"
       "Please include the generated file in your report.\n"
    );
 #endif
 
-   fprintf(stderr,
+   snprintf(err_buf, sizeof(err_buf),
       "Running this program with debug symbols or inside a debugger may provide further insights.\n"
       "\n"
       "Thank you for helping to improve %s!\n"
       "\n",
       program
    );
+   full_write_str(STDERR_FILENO, err_buf);
 
    /* Call old sigsegv handler; may be default exit or third party one (e.g. ASAN) */
    if (sigaction(signal, &old_sig_handler[signal], NULL) < 0) {
       /* This avoids an infinite loop in case the handler could not be reset. */
-      fprintf(stderr,
+      full_write_str(STDERR_FILENO,
          "!!! Chained handler could not be restored. Forcing exit.\n"
       );
       _exit(1);
@@ -1317,7 +1329,7 @@ void CRT_handleSIGSEGV(int signal) {
    raise(signal);
 
    // Always terminate, even if installed handler returns
-   fprintf(stderr,
+   full_write_str(STDERR_FILENO,
       "!!! Chained handler did not exit. Forcing exit.\n"
    );
    _exit(1);
diff --git a/Settings.c b/Settings.c
index ec79d242..95c30305 100644
--- a/Settings.c
+++ b/Settings.c
@@ -598,65 +598,88 @@ static bool Settings_read(Settings* this, const char* fileName, unsigned int ini
    return didReadAny;
 }
 
-static void writeFields(FILE* fd, const ProcessField* fields, Hashtable* columns, bool byName, char separator) {
+typedef ATTR_FORMAT(printf, 2, 3) int (*OutputFunc)(FILE*, const char*,...);
+
+static void writeFields(OutputFunc of, FILE* fd,
+                        const ProcessField* fields, Hashtable* columns,
+                        bool byName, char separator) {
    const char* sep = "";
    for (unsigned int i = 0; fields[i]; i++) {
       if (fields[i] < LAST_PROCESSFIELD && byName) {
          const char* pName = toFieldName(columns, fields[i], NULL);
-         fprintf(fd, "%s%s", sep, pName);
+         of(fd, "%s%s", sep, pName);
       } else if (fields[i] >= LAST_PROCESSFIELD && byName) {
          bool enabled;
          const char* pName = toFieldName(columns, fields[i], &enabled);
          if (enabled)
-            fprintf(fd, "%sDynamic(%s)", sep, pName);
+            of(fd, "%sDynamic(%s)", sep, pName);
       } else {
          // This "-1" is for compatibility with the older enum format.
-         fprintf(fd, "%s%d", sep, (int) fields[i] - 1);
+         of(fd, "%s%d", sep, (int) fields[i] - 1);
       }
       sep = " ";
    }
-   fputc(separator, fd);
+   of(fd, "%c", separator);
 }
 
-static void writeList(FILE* fd, char** list, int len, char separator) {
+static void writeList(OutputFunc of, FILE* fd,
+                      char** list, int len, char separator) {
    const char* sep = "";
    for (int i = 0; i < len; i++) {
-      fprintf(fd, "%s%s", sep, list[i]);
+      of(fd, "%s%s", sep, list[i]);
       sep = " ";
    }
-   fputc(separator, fd);
+   of(fd, "%c", separator);
 }
 
-static void writeMeters(const Settings* this, FILE* fd, char separator, unsigned int column) {
+static void writeMeters(const Settings* this, OutputFunc of,
+                        FILE* fd, char separator, unsigned int column) {
    if (this->hColumns[column].len) {
-      writeList(fd, this->hColumns[column].names, this->hColumns[column].len, separator);
+      writeList(of, fd, this->hColumns[column].names, this->hColumns[column].len, separator);
    } else {
-      fputc('!', fd);
-      fputc(separator, fd);
+      of(fd, "!%c", separator);
    }
 }
 
-static void writeMeterModes(const Settings* this, FILE* fd, char separator, unsigned int column) {
+static void writeMeterModes(const Settings* this, OutputFunc of,
+                            FILE* fd, char separator, unsigned int column) {
    if (this->hColumns[column].len) {
       const char* sep = "";
       for (size_t i = 0; i < this->hColumns[column].len; i++) {
-         fprintf(fd, "%s%d", sep, this->hColumns[column].modes[i]);
+         of(fd, "%s%d", sep, this->hColumns[column].modes[i]);
          sep = " ";
       }
    } else {
-      fputc('!', fd);
+      of(fd, "!");
    }
 
-   fputc(separator, fd);
+   of(fd, "%c", separator);
+}
+
+ATTR_FORMAT(printf, 2, 3)
+static int signal_safe_fprintf(FILE* stream, const char* fmt, ...) {
+   char buf[2048];
+
+   va_list vl;
+   va_start(vl, fmt);
+   int n = vsnprintf(buf, sizeof(buf), fmt, vl);
+   va_end(vl);
+
+   if (n <= 0)
+      return n;
+
+   return full_write_str(fileno(stream), buf);
 }
 
 int Settings_write(const Settings* this, bool onCrash) {
    FILE* fd;
    char separator;
    char* tmpFilename = NULL;
+   OutputFunc of;
    if (onCrash) {
       fd = stderr;
       separator = ';';
+      of = signal_safe_fprintf;
    } else if (!this->writeConfig) {
       return 0;
    } else {
@@ -671,20 +694,21 @@ int Settings_write(const Settings* this, bool onCrash) {
       if (fd == NULL)
          return -errno;
       separator = '\n';
+      of = fprintf;
    }
 
    #define printSettingInteger(setting_, value_) \
-      fprintf(fd, setting_ "=%d%c", (int) (value_), separator)
+      of(fd, setting_ "=%d%c", (int) (value_), separator)
    #define printSettingString(setting_, value_) \
-      fprintf(fd, setting_ "=%s%c", value_, separator)
+      of(fd, setting_ "=%s%c", value_, separator)
 
    if (!onCrash) {
-      fprintf(fd, "# Beware! This file is rewritten by htop when settings are changed in the interface.\n");
-      fprintf(fd, "# The parser is also very primitive, and not human-friendly.\n");
+      of(fd, "# Beware! This file is rewritten by htop when settings are changed in the interface.\n");
+      of(fd, "# The parser is also very primitive, and not human-friendly.\n");
    }
    printSettingString("htop_version", VERSION);
    printSettingInteger("config_reader_min_version", CONFIG_READER_MIN_VERSION);
-   fprintf(fd, "fields="); writeFields(fd, this->screens[0]->fields, this->dynamicColumns, false, separator);
+   of(fd, "fields="); writeFields(of, fd, this->screens[0]->fields, this->dynamicColumns, false, separator);
    printSettingInteger("hide_kernel_threads", this->hideKernelThreads);
    printSettingInteger("hide_userland_threads", this->hideUserlandThreads);
    printSettingInteger("hide_running_in_container", this->hideRunningInContainer);
@@ -725,10 +749,10 @@ int Settings_write(const Settings* this, bool onCrash) {
 
    printSettingString("header_layout", HeaderLayout_getName(this->hLayout));
    for (unsigned int i = 0; i < HeaderLayout_getColumns(this->hLayout); i++) {
-      fprintf(fd, "column_meters_%u=", i);
-      writeMeters(this, fd, separator, i);
-      fprintf(fd, "column_meter_modes_%u=", i);
-      writeMeterModes(this, fd, separator, i);
+      of(fd, "column_meters_%u=", i);
+      writeMeters(this, of, fd, separator, i);
+      of(fd, "column_meter_modes_%u=", i);
+      writeMeterModes(this, of, fd, separator, i);
    }
 
    // Legacy compatibility with older versions of htop
@@ -746,14 +770,14 @@ int Settings_write(const Settings* this, bool onCrash) {
       const char* sortKey = toFieldName(this->dynamicColumns, ss->sortKey, NULL);
       const char* treeSortKey = toFieldName(this->dynamicColumns, ss->treeSortKey, NULL);
 
-      fprintf(fd, "screen:%s=", ss->heading);
-      writeFields(fd, ss->fields, this->dynamicColumns, true, separator);
+      of(fd, "screen:%s=", ss->heading);
+      writeFields(of, fd, ss->fields, this->dynamicColumns, true, separator);
       if (ss->dynamic) {
          printSettingString(".dynamic", ss->dynamic);
          if (ss->sortKey && ss->sortKey != PID)
-            fprintf(fd, "%s=Dynamic(%s)%c", ".sort_key", sortKey, separator);
+            of(fd, "%s=Dynamic(%s)%c", ".sort_key", sortKey, separator);
          if (ss->treeSortKey && ss->treeSortKey != PID)
-            fprintf(fd, "%s=Dynamic(%s)%c", ".tree_sort_key", treeSortKey, separator);
+            of(fd, "%s=Dynamic(%s)%c", ".tree_sort_key", treeSortKey, separator);
       } else {
          printSettingString(".sort_key", sortKey);
          printSettingString(".tree_sort_key", treeSortKey);
diff --git a/XUtils.h b/XUtils.h
index 32bc282d..3703f8bc 100644
--- a/XUtils.h
+++ b/XUtils.h
@@ -117,6 +117,10 @@ ssize_t xReadfileat(openat_arg_t dirfd, const char* pathname, void* buffer, size
 ATTR_ACCESS3_R(2, 3)
 ssize_t full_write(int fd, const void* buf, size_t count);
 
+static inline ssize_t full_write_str(int fd, const char* str) {
+   return full_write(fd, str, strlen(str));
+}
+
 /* Compares floating point values for ordering data entries. In this function,
    NaN is considered "less than" any other floating point value (regardless of
    sign), and two NaNs are considered "equal" regardless of payload. */