From bd91d4623ec4bc5780c16225c52cb42824b24038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sun, 23 Apr 2023 19:54:05 +0200 Subject: [PATCH 1/2] Add framework to show warnings on screen Some interactive user action might fail, e.g. due to lacking permissions. Those failures are currently hidden from the user. Add a framework to register warning messages to be displayed for a pre-defined amount of time on the screen. --- DisplayOptionsPanel.c | 1 + Makefile.am | 2 + ScreenManager.c | 3 + ScreenWarning.c | 129 ++++++++++++++++++++++++++++++++++++++++++ ScreenWarning.h | 18 ++++++ Settings.c | 4 ++ Settings.h | 1 + 7 files changed, 158 insertions(+) create mode 100644 ScreenWarning.c create mode 100644 ScreenWarning.h diff --git a/DisplayOptionsPanel.c b/DisplayOptionsPanel.c index f9fa9b12b..de24edcc6 100644 --- a/DisplayOptionsPanel.c +++ b/DisplayOptionsPanel.c @@ -128,6 +128,7 @@ DisplayOptionsPanel* DisplayOptionsPanel_new(Settings* settings, ScreenManager* Panel_add(super, (Object*) CheckItem_newByRef("- Try to strip exe from cmdline (when Command is merged)", &(settings->stripExeFromCmdline))); Panel_add(super, (Object*) CheckItem_newByRef("Highlight large numbers in memory counters", &(settings->highlightMegabytes))); Panel_add(super, (Object*) CheckItem_newByRef("Leave a margin around header", &(settings->headerMargin))); + Panel_add(super, (Object*) CheckItem_newByRef("Show warnings of failed interactive actions", &(settings->showWarnings))); Panel_add(super, (Object*) CheckItem_newByRef("Detailed CPU time (System/IO-Wait/Hard-IRQ/Soft-IRQ/Steal/Guest)", &(settings->detailedCPUTime))); Panel_add(super, (Object*) CheckItem_newByRef("Count CPUs from 1 instead of 0", &(settings->countCPUsFromOne))); Panel_add(super, (Object*) CheckItem_newByRef("Update process names on every refresh", &(settings->updateProcessNames))); diff --git a/Makefile.am b/Makefile.am index 427342014..bf2bc54b6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -77,6 +77,7 @@ myhtopsources = \ Scheduling.c \ ScreenManager.c \ ScreensPanel.c \ + ScreenWarning.c \ Settings.c \ SignalsPanel.c \ SwapMeter.c \ @@ -140,6 +141,7 @@ myhtopheaders = \ RichString.h \ Scheduling.h \ ScreenManager.h \ + ScreenWarning.h \ ScreensPanel.h \ Settings.h \ SignalsPanel.h \ diff --git a/ScreenManager.c b/ScreenManager.c index 87e718926..4028f7786 100644 --- a/ScreenManager.c +++ b/ScreenManager.c @@ -21,6 +21,7 @@ in the source distribution for its full text. #include "Platform.h" #include "ProcessList.h" #include "ProvideCurses.h" +#include "ScreenWarning.h" #include "XUtils.h" @@ -210,6 +211,8 @@ static void ScreenManager_drawPanels(ScreenManager* this, int focus, bool force_ State_hideFunctionBar(this->state)); mvvline(panel->y, panel->x + panel->w, ' ', panel->h + (State_hideFunctionBar(this->state) ? 1 : 0)); } + + ScreenWarning_display(); } void ScreenManager_run(ScreenManager* this, Panel** lastFocus, int* lastKey, const char* name) { diff --git a/ScreenWarning.c b/ScreenWarning.c new file mode 100644 index 000000000..d7fd2ba19 --- /dev/null +++ b/ScreenWarning.c @@ -0,0 +1,129 @@ +/* +htop - ScreenWarning.c +(C) 2023 htop dev team +Released under the GNU GPLv2+, see the COPYING file +in the source distribution for its full text. +*/ + +#include "ScreenWarning.h" + +#include "CRT.h" +#include "Platform.h" +#include "RichString.h" +#include "XUtils.h" + + +#define WARNING_TIMEOUT 5000 + + +static char* warnMsg; +static int warnMsgLen; +static uint64_t warnTime; +static bool shown; + +void ScreenWarning_add(const char* fmt, ...) { + free(warnMsg); + + va_list vl; + va_start(vl, fmt); + int r = vasprintf(&warnMsg, fmt, vl); + va_end(vl); + + assert(r >= 0); + assert(*warnMsg); + warnMsgLen = r; + + shown = false; +} + +#define DIV_ROUNDUP(a, b) (assert((b) != 0), ((a) + ((b) - 1)) / (b)) + +static void draw(void) { + const int width_padding = 2 * 5; // " [!] " + const int box_width = MINIMUM(warnMsgLen + width_padding, MINIMUM(4 * COLS / 5, 80)); + const int box_height = 2 + DIV_ROUNDUP(warnMsgLen, box_width - width_padding); + + const int col_start = (COLS - box_width) / 2; + const int line_start = LINES - box_height - (LINES < 20 ? 1 : 2); + + RichString_begin(rs); + + RichString_writeAscii(&rs, CRT_colors[PROCESS_D_STATE], " [!]"); + for (int i = 0; i <= box_width - width_padding; i++) { + switch (i % 3) { + case 0: + case 1: + RichString_appendChr(&rs, 0, ' ', 1); + break; + case 2: + RichString_appendChr(&rs, CRT_colors[METER_VALUE_NOTICE], '-', 1); + break; + } + } + RichString_appendChr(&rs, 0, ' ', 1); + RichString_appendAscii(&rs, CRT_colors[PROCESS_D_STATE], "[!] "); + RichString_printVal(rs, line_start, col_start); + + { + const char* remainMsg = warnMsg; + int remainMsgLen = warnMsgLen; + for (int i = 0; i < box_height - 2; i++) { + + RichString_rewind(&rs, RichString_sizeVal(rs)); + + RichString_writeAscii(&rs, CRT_colors[PROCESS_D_STATE], " [!]"); + RichString_appendChr(&rs, 0, ' ', 1); + int charsToPrint = MINIMUM(remainMsgLen, box_width - width_padding); + RichString_appendnAscii(&rs, CRT_colors[METER_VALUE_NOTICE], remainMsg, charsToPrint); + remainMsg += charsToPrint; + remainMsgLen -= charsToPrint; + RichString_appendChr(&rs, 0, ' ', 1 + box_width - width_padding - charsToPrint); + RichString_appendAscii(&rs, CRT_colors[PROCESS_D_STATE], "[!] "); + + RichString_printVal(rs, line_start + i + 1, col_start); + } + assert(*remainMsg == '\0'); + assert(remainMsgLen == 0); + } + + RichString_rewind(&rs, RichString_sizeVal(rs)); + + RichString_writeAscii(&rs, CRT_colors[PROCESS_D_STATE], " [!]"); + for (int i = 0; i <= box_width - width_padding; i++) { + switch (i % 3) { + case 0: + case 1: + RichString_appendChr(&rs, 0, ' ', 1); + break; + case 2: + RichString_appendChr(&rs, CRT_colors[METER_VALUE_NOTICE], '-', 1); + break; + } + } + RichString_appendChr(&rs, 0, ' ', 1); + RichString_appendAscii(&rs, CRT_colors[PROCESS_D_STATE], "[!] "); + RichString_printVal(rs, line_start + box_height - 1, col_start); + + RichString_delete(&rs); +} + +void ScreenWarning_display(void) { + if (!warnMsg) + return; + + uint64_t now; + Platform_gettime_monotonic(&now); + + if (shown && now >= (warnTime + WARNING_TIMEOUT)) { + free(warnMsg); + warnMsg = NULL; + return; + } + + draw(); + + if (!shown) { + warnTime = now; + shown = true; + } +} diff --git a/ScreenWarning.h b/ScreenWarning.h new file mode 100644 index 000000000..c05eb6b8a --- /dev/null +++ b/ScreenWarning.h @@ -0,0 +1,18 @@ +#ifndef HEADER_ScreenWarning +#define HEADER_ScreenWarning +/* +htop - ScreenWarning.h +(C) 2023 htop dev team +Released under the GNU GPLv2+, see the COPYING file +in the source distribution for its full text. +*/ + +#include "Macros.h" + + +ATTR_FORMAT(printf, 1, 2) +void ScreenWarning_add(const char* fmt, ...); + +void ScreenWarning_display(void); + +#endif /* HEADER_ScreenWarning */ diff --git a/Settings.c b/Settings.c index c712966e3..2c8746a92 100644 --- a/Settings.c +++ b/Settings.c @@ -411,6 +411,8 @@ static bool Settings_read(Settings* this, const char* fileName, unsigned int ini this->showMergedCommand = atoi(option[1]); } else if (String_eq(option[0], "header_margin")) { this->headerMargin = atoi(option[1]); + } else if (String_eq(option[0], "show_warnings")) { + this->showWarnings = atoi(option[1]); } else if (String_eq(option[0], "screen_tabs")) { this->screenTabs = atoi(option[1]); } else if (String_eq(option[0], "expand_system_time")) { @@ -598,6 +600,7 @@ int Settings_write(const Settings* this, bool onCrash) { printSettingInteger("strip_exe_from_cmdline", this->stripExeFromCmdline); printSettingInteger("show_merged_command", this->showMergedCommand); printSettingInteger("header_margin", this->headerMargin); + printSettingInteger("show_warnings", this->showWarnings); printSettingInteger("screen_tabs", this->screenTabs); printSettingInteger("detailed_cpu_time", this->detailedCPUTime); printSettingInteger("cpu_count_from_one", this->countCPUsFromOne); @@ -702,6 +705,7 @@ Settings* Settings_new(unsigned int initialCpuCount, Hashtable* dynamicMeters, H this->showMergedCommand = false; this->hideFunctionBar = 0; this->headerMargin = true; + this->showWarnings = true; #ifdef HAVE_LIBHWLOC this->topologyAffinity = false; #endif diff --git a/Settings.h b/Settings.h index 48c62590a..9c9a429de 100644 --- a/Settings.h +++ b/Settings.h @@ -89,6 +89,7 @@ typedef struct Settings_ { bool updateProcessNames; bool accountGuestInCPUMeter; bool headerMargin; + bool showWarnings; bool screenTabs; #ifdef HAVE_GETMOUSE bool enableMouse; From 855eee75ee80faa4e3fcf28485656039a33421ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sun, 23 Apr 2023 19:54:22 +0200 Subject: [PATCH 2/2] Show warnings on failures of interactive user actions --- Affinity.c | 27 +++++++++++++++++++-------- Process.c | 10 +++++++++- Scheduling.c | 3 +++ linux/LinuxProcess.c | 8 +++++++- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/Affinity.c b/Affinity.c index dc6a7fba0..2d2d44eb3 100644 --- a/Affinity.c +++ b/Affinity.c @@ -10,8 +10,10 @@ in the source distribution for its full text. #include "Affinity.h" +#include #include +#include "ScreenWarning.h" #include "XUtils.h" #if defined(HAVE_LIBHWLOC) @@ -54,9 +56,11 @@ void Affinity_add(Affinity* this, unsigned int id) { Affinity* Affinity_get(const Process* proc, ProcessList* pl) { hwloc_cpuset_t cpuset = hwloc_bitmap_alloc(); - bool ok = (hwloc_get_proc_cpubind(pl->topology, proc->pid, cpuset, HTOP_HWLOC_CPUBIND_FLAG) == 0); + int r = hwloc_get_proc_cpubind(pl->topology, proc->pid, cpuset, HTOP_HWLOC_CPUBIND_FLAG); + if (r < 0) + ScreenWarning_add("Failed to get affinity for process %d (%s): %s", proc->pid, proc->procComm, strerror(errno)); Affinity* affinity = NULL; - if (ok) { + if (r == 0) { affinity = Affinity_new(pl); if (hwloc_bitmap_last(cpuset) == -1) { for (unsigned int i = 0; i < pl->existingCPUs; i++) { @@ -79,18 +83,22 @@ bool Affinity_set(Process* proc, Arg arg) { for (unsigned int i = 0; i < this->used; i++) { hwloc_bitmap_set(cpuset, this->cpus[i]); } - bool ok = (hwloc_set_proc_cpubind(this->pl->topology, proc->pid, cpuset, HTOP_HWLOC_CPUBIND_FLAG) == 0); + int r = hwloc_set_proc_cpubind(this->pl->topology, proc->pid, cpuset, HTOP_HWLOC_CPUBIND_FLAG); + if (r < 0) + ScreenWarning_add("Failed to set affinity for process %d (%s): %s", proc->pid, proc->procComm, strerror(errno)); hwloc_bitmap_free(cpuset); - return ok; + return r == 0; } #elif defined(HAVE_AFFINITY) Affinity* Affinity_get(const Process* proc, ProcessList* pl) { cpu_set_t cpuset; - bool ok = (sched_getaffinity(proc->pid, sizeof(cpu_set_t), &cpuset) == 0); - if (!ok) + int r = sched_getaffinity(proc->pid, sizeof(cpu_set_t), &cpuset); + if (r < 0) { + ScreenWarning_add("Failed to get affinity for process %d (%s): %s", proc->pid, proc->procComm, strerror(errno)); return NULL; + } Affinity* affinity = Affinity_new(pl); for (unsigned int i = 0; i < pl->existingCPUs; i++) { @@ -108,8 +116,11 @@ bool Affinity_set(Process* proc, Arg arg) { for (unsigned int i = 0; i < this->used; i++) { CPU_SET(this->cpus[i], &cpuset); } - bool ok = (sched_setaffinity(proc->pid, sizeof(unsigned long), &cpuset) == 0); - return ok; + int r = sched_setaffinity(proc->pid, sizeof(unsigned long), &cpuset); + if (r < 0) + ScreenWarning_add("Failed to set affinity for process %d (%s): %s", proc->pid, proc->procComm, strerror(errno)); + + return r == 0; } #endif diff --git a/Process.c b/Process.c index c92d01b99..6f11e90da 100644 --- a/Process.c +++ b/Process.c @@ -11,6 +11,7 @@ in the source distribution for its full text. #include "Process.h" #include +#include #include #include #include @@ -29,6 +30,7 @@ in the source distribution for its full text. #include "DynamicColumn.h" #include "RichString.h" #include "Scheduling.h" +#include "ScreenWarning.h" #include "Settings.h" #include "XUtils.h" @@ -1134,6 +1136,8 @@ bool Process_setPriority(Process* this, int priority) { int old_prio = getpriority(PRIO_PROCESS, this->pid); int err = setpriority(PRIO_PROCESS, this->pid, priority); + if (err < 0) + ScreenWarning_add("Failed to set priority %d for process %d (%s): %s", priority, this->pid, this->procComm, strerror(errno)); if (err == 0 && old_prio != getpriority(PRIO_PROCESS, this->pid)) { this->nice = priority; @@ -1146,7 +1150,11 @@ bool Process_changePriorityBy(Process* this, Arg delta) { } bool Process_sendSignal(Process* this, Arg sgn) { - return kill(this->pid, sgn.i) == 0; + int r = kill(this->pid, sgn.i); + if (r < 0) + ScreenWarning_add("Failed to send signal %d to process %d (%s): %s", sgn.i, this->pid, this->procComm, strerror(errno)); + + return r == 0; } int Process_compare(const void* v1, const void* v2) { diff --git a/Scheduling.c b/Scheduling.c index 5ca49ae2d..44a702f55 100644 --- a/Scheduling.c +++ b/Scheduling.c @@ -17,6 +17,7 @@ in the source distribution for its full text. #include "Macros.h" #include "Object.h" #include "Panel.h" +#include "ScreenWarning.h" #include "XUtils.h" @@ -113,6 +114,8 @@ bool Scheduling_setPolicy(Process* proc, Arg arg) { #endif int r = sched_setscheduler(proc->pid, policy, ¶m); + if (r == -1) + ScreenWarning_add("Failed to set scheduling policy %d for process %d (%s): %s", policy, proc->pid, proc->procComm, strerror(errno)); /* POSIX says on success the previous scheduling policy should be returned, * but Linux always returns 0. */ diff --git a/linux/LinuxProcess.c b/linux/LinuxProcess.c index 8f9c34620..ab6391869 100644 --- a/linux/LinuxProcess.c +++ b/linux/LinuxProcess.c @@ -8,6 +8,7 @@ in the source distribution for its full text. #include "linux/LinuxProcess.h" +#include #include #include #include @@ -20,6 +21,7 @@ in the source distribution for its full text. #include "ProvideCurses.h" #include "RichString.h" #include "Scheduling.h" +#include "ScreenWarning.h" #include "XUtils.h" #include "linux/IOPriority.h" @@ -159,7 +161,9 @@ IOPriority LinuxProcess_updateIOPriority(LinuxProcess* this) { bool LinuxProcess_setIOPriority(Process* this, Arg ioprio) { // Other OSes masquerading as Linux (NetBSD?) don't have this syscall #ifdef SYS_ioprio_set - syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, this->pid, ioprio.i); + int r = syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, this->pid, ioprio.i); + if (r < 0) + ScreenWarning_add("Failed to set IO priority %d for process %d (%s): %s", ioprio.i, this->pid, this->procComm, strerror(errno)); #endif return (LinuxProcess_updateIOPriority((LinuxProcess*)this) == ioprio.i); } @@ -187,6 +191,8 @@ bool LinuxProcess_changeAutogroupPriorityBy(Process* this, Arg delta) { rewind(file); xSnprintf(buffer, sizeof(buffer), "%d", nice + delta.i); success = fputs(buffer, file) > 0; + if (!success) + ScreenWarning_add("Failed to set autogroup for process %d (%s): %s", this->pid, this->procComm, strerror(errno)); } else { success = false; }