Process Field Rework

AvailableColumnsPanel.c

@@ -77,7 +77,7 @@ AvailableColumnsPanel* AvailableColumnsPanel_new(Panel* columns) {
 
    Panel_setHeader(super, "Available Columns");
 
-   for (int i = 1; i < Platform_numberOfFields; i++) {
+   for (int i = 1; i < LAST_PROCESSFIELD; i++) {
       if (i != COMM && Process_fields[i].description) {
          char description[256];
          xSnprintf(description, sizeof(description), "%s - %s", Process_fields[i].name, Process_fields[i].description);

BatteryMeter.c

@@ -35,24 +35,21 @@ static void BatteryMeter_updateValues(Meter* this, char* buffer, size_t len) {
 
    this->values[0] = percent;
 
-   const char *onAcText, *onBatteryText, *unknownText;
-
-   unknownText = "%.1f%%";
-   if (this->mode == TEXT_METERMODE) {
-      onAcText = "%.1f%% (Running on A/C)";
-      onBatteryText = "%.1f%% (Running on battery)";
-   } else {
-      onAcText = "%.1f%%(A/C)";
-      onBatteryText = "%.1f%%(bat)";
+   const char* text;
+   switch (isOnAC) {
+   case AC_PRESENT:
+      text = this->mode == TEXT_METERMODE ? " (Running on A/C)" : "(A/C)";
+      break;
+   case AC_ABSENT:
+      text = this->mode == TEXT_METERMODE ? " (Running on battery)" : "(bat)";
+      break;
+   case AC_ERROR:
+   default:
+      text = "";
+      break;
    }
 
-   if (isOnAC == AC_PRESENT) {
-      xSnprintf(buffer, len, onAcText, percent);
-   } else if (isOnAC == AC_ABSENT) {
-      xSnprintf(buffer, len, onBatteryText, percent);
-   } else {
-      xSnprintf(buffer, len, unknownText, percent);
-   }
+   xSnprintf(buffer, len, "%.1f%%%s", percent, text);
 }
 
 const MeterClass BatteryMeter_class = {

Makefile.am

@@ -133,6 +133,7 @@ linux_platform_headers = \
 	linux/LinuxProcessList.h \
 	linux/Platform.h \
 	linux/PressureStallMeter.h \
+	linux/ProcessField.h \
 	linux/SELinuxMeter.h \
 	linux/SystemdMeter.h \
 	linux/ZramMeter.h \
@@ -164,9 +165,10 @@ endif
 # -------
 
 freebsd_platform_headers = \
-	freebsd/Platform.h \
 	freebsd/FreeBSDProcessList.h \
 	freebsd/FreeBSDProcess.h \
+	freebsd/Platform.h \
+	freebsd/ProcessField.h \
 	zfs/ZfsArcMeter.h \
 	zfs/ZfsCompressedArcMeter.h \
 	zfs/ZfsArcStats.h \
@@ -184,9 +186,10 @@ endif
 # ------------
 
 dragonflybsd_platform_headers = \
-	dragonflybsd/Platform.h \
 	dragonflybsd/DragonFlyBSDProcessList.h \
-	dragonflybsd/DragonFlyBSDProcess.h
+	dragonflybsd/DragonFlyBSDProcess.h \
+	dragonflybsd/Platform.h \
+	dragonflybsd/ProcessField.h
 
 if HTOP_DRAGONFLYBSD
 AM_LDFLAGS += -lkvm -lkinfo
@@ -200,9 +203,10 @@ endif
 # -------
 
 openbsd_platform_headers = \
-	openbsd/Platform.h \
 	openbsd/OpenBSDProcessList.h \
-	openbsd/OpenBSDProcess.h
+	openbsd/OpenBSDProcess.h \
+	openbsd/Platform.h \
+	openbsd/ProcessField.h
 
 if HTOP_OPENBSD
 myhtopplatsources = openbsd/Platform.c openbsd/OpenBSDProcessList.c \
@@ -215,9 +219,10 @@ endif
 # ------
 
 darwin_platform_headers = \
-	darwin/Platform.h \
 	darwin/DarwinProcess.h \
 	darwin/DarwinProcessList.h \
+	darwin/Platform.h \
+	darwin/ProcessField.h \
 	zfs/ZfsArcMeter.h \
 	zfs/ZfsCompressedArcMeter.h \
 	zfs/ZfsArcStats.h \
@@ -237,6 +242,7 @@ endif
 
 solaris_platform_headers = \
 	solaris/Platform.h \
+	solaris/ProcessField.h \
 	solaris/SolarisProcess.h \
 	solaris/SolarisProcessList.h \
 	zfs/ZfsArcMeter.h \
@@ -256,6 +262,7 @@ endif
 
 unsupported_platform_headers = \
 	unsupported/Platform.h \
+	unsupported/ProcessField.h \
 	unsupported/UnsupportedProcess.h \
 	unsupported/UnsupportedProcessList.h
 

Process.c

@@ -38,23 +38,15 @@ in the source distribution for its full text.
 
 static uid_t Process_getuid = (uid_t)-1;
 
-char Process_pidFormat[20] = "%7d ";
-
-static char Process_titleBuffer[20][20];
+int Process_pidDigits = 7;
 
 void Process_setupColumnWidths() {
    int maxPid = Platform_getMaxPid();
    if (maxPid == -1)
       return;
 
-   int digits = ceil(log10(maxPid));
-   assert(digits < 20);
-   for (int i = 0; Process_pidColumns[i].label; i++) {
-      assert(i < 20);
-      xSnprintf(Process_titleBuffer[i], 20, "%*s ", digits, Process_pidColumns[i].label);
-      Process_fields[Process_pidColumns[i].id].title = Process_titleBuffer[i];
-   }
-   xSnprintf(Process_pidFormat, sizeof(Process_pidFormat), "%%%dd ", digits);
+   Process_pidDigits = ceil(log10(maxPid));
+   assert(Process_pidDigits <= PROCESS_MAX_PID_DIGITS);
 }
 
 void Process_humanNumber(RichString* str, unsigned long long number, bool coloring) {
@@ -338,9 +330,9 @@ void Process_writeField(const Process* this, RichString* str, ProcessField field
       break;
    }
    case NLWP: xSnprintf(buffer, n, "%4ld ", this->nlwp); break;
-   case PGRP: xSnprintf(buffer, n, Process_pidFormat, this->pgrp); break;
-   case PID: xSnprintf(buffer, n, Process_pidFormat, this->pid); break;
-   case PPID: xSnprintf(buffer, n, Process_pidFormat, this->ppid); break;
+   case PGRP: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->pgrp); break;
+   case PID: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->pid); break;
+   case PPID: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->ppid); break;
    case PRIORITY: {
       if(this->priority <= -100)
          xSnprintf(buffer, n, " RT ");
@@ -349,7 +341,7 @@ void Process_writeField(const Process* this, RichString* str, ProcessField field
       break;
    }
    case PROCESSOR: xSnprintf(buffer, n, "%3d ", Settings_cpuId(this->settings, this->processor)); break;
-   case SESSION: xSnprintf(buffer, n, Process_pidFormat, this->session); break;
+   case SESSION: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->session); break;
    case STARTTIME: xSnprintf(buffer, n, "%s", this->starttime_show); break;
    case STATE: {
       xSnprintf(buffer, n, "%c ", this->state);
@@ -365,8 +357,8 @@ void Process_writeField(const Process* this, RichString* str, ProcessField field
    }
    case ST_UID: xSnprintf(buffer, n, "%5d ", this->st_uid); break;
    case TIME: Process_printTime(str, this->time); return;
-   case TGID: xSnprintf(buffer, n, Process_pidFormat, this->tgid); break;
-   case TPGID: xSnprintf(buffer, n, Process_pidFormat, this->tpgid); break;
+   case TGID: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->tgid); break;
+   case TPGID: xSnprintf(buffer, n, "%*d ", Process_pidDigits, this->tpgid); break;
    case TTY_NR: xSnprintf(buffer, n, "%3u:%3u ", major(this->tty_nr), minor(this->tty_nr)); break;
    case USER: {
       if (Process_getuid != this->st_uid)
@@ -518,7 +510,7 @@ long Process_compare(const void* v1, const void* v2) {
 long Process_compareByKey_Base(const Process* p1, const Process* p2, ProcessField key) {
    int r;
 
-   switch ((int) key) {
+   switch (key) {
    case PERCENT_CPU:
    case PERCENT_NORM_CPU:
       return SPACESHIP_NUMBER(p2->percent_cpu, p1->percent_cpu);

Process.h

@@ -13,6 +13,7 @@ in the source distribution for its full text.
 #include <sys/types.h>
 
 #include "Object.h"
+#include "ProcessField.h"
 #include "RichString.h"
 
 
@@ -45,12 +46,12 @@ typedef enum ProcessField_ {
    NLWP = 51,
    TGID = 52,
    PERCENT_NORM_CPU = 53,
-} ProcessField;
 
-typedef struct ProcessPidColumn_ {
-   int id;
-   const char* label;
-} ProcessPidColumn;
+   /* Platform specific fields, defined in ${platform}/ProcessField.h */
+   PLATFORM_PROCESS_FIELDS
+
+   LAST_PROCESSFIELD
+} ProcessField;
 
 struct Settings_;
 
@@ -116,16 +117,17 @@ typedef struct ProcessFieldData_ {
    const char* title;
    const char* description;
    uint32_t flags;
+   bool pidColumn;
 } ProcessFieldData;
 
 // Implemented in platform-specific code:
 void Process_writeField(const Process* this, RichString* str, ProcessField field);
 long Process_compare(const void* v1, const void* v2);
 void Process_delete(Object* cast);
 bool Process_isThread(const Process* this);
-extern ProcessFieldData Process_fields[];
-extern ProcessPidColumn Process_pidColumns[];
-extern char Process_pidFormat[20];
+extern const ProcessFieldData Process_fields[LAST_PROCESSFIELD];
+#define PROCESS_MAX_PID_DIGITS 19
+extern int Process_pidDigits;
 
 typedef Process*(*Process_New)(const struct Settings_*);
 typedef void (*Process_WriteField)(const Process*, RichString*, ProcessField);

ProcessList.c

@@ -79,6 +79,20 @@ void ProcessList_setPanel(ProcessList* this, Panel* panel) {
    this->panel = panel;
 }
 
+static const char* alignedProcessFieldTitle(ProcessField field) {
+   const char* title = Process_fields[field].title;
+   if (!title)
+      return "- ";
+
+   if (!Process_fields[field].pidColumn)
+      return title;
+
+   static char titleBuffer[PROCESS_MAX_PID_DIGITS + /* space */ 1 + /* null-terminator */ + 1];
+   xSnprintf(titleBuffer, sizeof(titleBuffer), "%*s ", Process_pidDigits, title);
+
+   return titleBuffer;
+}
+
 void ProcessList_printHeader(ProcessList* this, RichString* header) {
    RichString_prune(header);
 
@@ -88,11 +102,6 @@ void ProcessList_printHeader(ProcessList* this, RichString* header) {
    ProcessField key = Settings_getActiveSortKey(settings);
 
    for (int i = 0; fields[i]; i++) {
-      const char* field = Process_fields[fields[i]].title;
-      if (!field) {
-         field = "- ";
-      }
-
       int color;
       if (settings->treeView && settings->treeViewAlwaysByPID) {
          color = CRT_colors[PANEL_HEADER_FOCUS];
@@ -102,7 +111,7 @@ void ProcessList_printHeader(ProcessList* this, RichString* header) {
          color = CRT_colors[PANEL_HEADER_FOCUS];
       }
 
-      RichString_appendWide(header, color, field);
+      RichString_appendWide(header, color, alignedProcessFieldTitle(fields[i]));
       if (COMM == fields[i] && settings->showMergedCommand) {
          RichString_appendAscii(header, color, "(merged)");
       }
@@ -456,12 +465,7 @@ ProcessField ProcessList_keyAt(const ProcessList* this, int at) {
    const ProcessField* fields = this->settings->fields;
    ProcessField field;
    for (int i = 0; (field = fields[i]); i++) {
-      const char* title = Process_fields[field].title;
-      if (!title) {
-         title = "- ";
-      }
-
-      int len = strlen(title);
+      int len = strlen(alignedProcessFieldTitle(field));
       if (at >= x && at <= x + len) {
          return field;
       }

Settings.c

@@ -96,10 +96,10 @@ static void readFields(ProcessField* fields, uint32_t* flags, const char* line)
    free(trim);
    int i, j;
    *flags = 0;
-   for (j = 0, i = 0; i < Platform_numberOfFields && ids[i]; i++) {
+   for (j = 0, i = 0; i < LAST_PROCESSFIELD && ids[i]; i++) {
       // This "+1" is for compatibility with the older enum format.
       int id = atoi(ids[i]) + 1;
-      if (id > 0 && id < Platform_numberOfFields && Process_fields[id].name) {
+      if (id > 0 && id < LAST_PROCESSFIELD && Process_fields[id].name) {
          fields[j] = id;
          *flags |= Process_fields[id].flags;
          j++;
@@ -355,11 +355,11 @@ Settings* Settings_new(int initialCpuCount) {
    #ifdef HAVE_LIBHWLOC
    this->topologyAffinity = false;
    #endif
-   this->fields = xCalloc(Platform_numberOfFields + 1, sizeof(ProcessField));
+   this->fields = xCalloc(LAST_PROCESSFIELD + 1, sizeof(ProcessField));
    // TODO: turn 'fields' into a Vector,
    // (and ProcessFields into proper objects).
    this->flags = 0;
-   ProcessField* defaults = Platform_defaultFields;
+   const ProcessField* defaults = Platform_defaultFields;
    for (int i = 0; defaults[i]; i++) {
       this->fields[i] = defaults[i];
       this->flags |= Process_fields[defaults[i]].flags;

configure.ac

@@ -333,6 +333,7 @@ AM_CFLAGS="\
  -Wcast-qual\
  -Wextra\
  -Wfloat-equal\
+ -Wformat=2\
  -Wmissing-format-attribute\
  -Wmissing-noreturn\
  -Wmissing-prototypes\

darwin/DarwinProcess.c

@@ -18,16 +18,16 @@ in the source distribution for its full text.
 #include "Process.h"
 
 
-ProcessFieldData Process_fields[] = {
+const ProcessFieldData Process_fields[LAST_PROCESSFIELD] = {
    [0] = { .name = "", .title = NULL, .description = NULL, .flags = 0, },
-   [PID] = { .name = "PID", .title = "    PID ", .description = "Process/thread ID", .flags = 0, },
+   [PID] = { .name = "PID", .title = "PID", .description = "Process/thread ID", .flags = 0, .pidColumn = true, },
    [COMM] = { .name = "Command", .title = "Command ", .description = "Command line", .flags = 0, },
    [STATE] = { .name = "STATE", .title = "S ", .description = "Process state (S sleeping, R running, D disk, Z zombie, T traced, W paging)", .flags = 0, },
-   [PPID] = { .name = "PPID", .title = "   PPID ", .description = "Parent process ID", .flags = 0, },
-   [PGRP] = { .name = "PGRP", .title = "   PGRP ", .description = "Process group ID", .flags = 0, },
-   [SESSION] = { .name = "SESSION", .title = "    SID ", .description = "Process's session ID", .flags = 0, },
+   [PPID] = { .name = "PPID", .title = "PPID", .description = "Parent process ID", .flags = 0, .pidColumn = true, },
+   [PGRP] = { .name = "PGRP", .title = "PGRP", .description = "Process group ID", .flags = 0, .pidColumn = true, },
+   [SESSION] = { .name = "SESSION", .title = "SID", .description = "Process's session ID", .flags = 0, .pidColumn = true, },
    [TTY_NR] = { .name = "TTY_NR", .title = "    TTY ", .description = "Controlling terminal", .flags = 0, },
-   [TPGID] = { .name = "TPGID", .title = "  TPGID ", .description = "Process ID of the fg process group of the controlling terminal", .flags = 0, },
+   [TPGID] = { .name = "TPGID", .title = "TPGID", .description = "Process ID of the fg process group of the controlling terminal", .flags = 0, .pidColumn = true, },
    [MINFLT] = { .name = "MINFLT", .title = "     MINFLT ", .description = "Number of minor faults which have not required loading a memory page from disk", .flags = 0, },
    [MAJFLT] = { .name = "MAJFLT", .title = "     MAJFLT ", .description = "Number of major faults which have required loading a memory page from disk", .flags = 0, },
    [PRIORITY] = { .name = "PRIORITY", .title = "PRI ", .description = "Kernel's internal priority for the process", .flags = 0, },
@@ -43,9 +43,8 @@ ProcessFieldData Process_fields[] = {
    [USER] = { .name = "USER", .title = "USER      ", .description = "Username of the process owner (or user ID if name cannot be determined)", .flags = 0, },
    [TIME] = { .name = "TIME", .title = "  TIME+  ", .description = "Total time the process has spent in user and system time", .flags = 0, },
    [NLWP] = { .name = "NLWP", .title = "NLWP ", .description = "Number of threads in the process", .flags = 0, },
-   [TGID] = { .name = "TGID", .title = "   TGID ", .description = "Thread group ID (i.e. process ID)", .flags = 0, },
+   [TGID] = { .name = "TGID", .title = "TGID", .description = "Thread group ID (i.e. process ID)", .flags = 0, .pidColumn = true, },
    [TRANSLATED] = { .name = "TRANSLATED", .title = "T ", .description = "Translation info (T translated, N native)", .flags = 0, },
-   [LAST_PROCESSFIELD] = { .name = "*** report bug! ***", .title = NULL, .description = NULL, .flags = 0, },
 };
 
 Process* DarwinProcess_new(const Settings* settings) {
@@ -73,7 +72,7 @@ static void DarwinProcess_writeField(const Process* this, RichString* str, Proce
    char buffer[256]; buffer[255] = '\0';
    int attr = CRT_colors[DEFAULT_COLOR];
    int n = sizeof(buffer) - 1;
-   switch ((int) field) {
+   switch (field) {
    // add Platform-specific fields here
    case TRANSLATED: xSnprintf(buffer, n, "%c ", dp->translated ? 'T' : 'N'); break;
    default:
@@ -87,7 +86,7 @@ static long DarwinProcess_compareByKey(const Process* v1, const Process* v2, Pro
    const DarwinProcess* p1 = (const DarwinProcess*)v1;
    const DarwinProcess* p2 = (const DarwinProcess*)v2;
 
-   switch ((int) key) {
+   switch (key) {
    // add Platform-specific fields here
    case TRANSLATED:
       return SPACESHIP_NUMBER(p1->translated, p2->translated);