From 397b5c4bd09115d0df0846fee1b06797b68ae11c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Wed, 4 Nov 2020 17:46:24 +0100 Subject: [PATCH] Introduce spaceship comparison for Processes If currently two unsigned values are compared via `a - b`, in the case b is actually bigger than a, the result will not be an negative number (as -1 is expected) but a huge positive number as the subtraction is an unsigned subtraction. Avoid over-/underflow affected operations; use comparisons. Modern compilers will generate sane code, like: xor eax, eax cmp rdi, rsi seta al sbb eax, 0 ret --- Macros.h | 16 ++++-- Process.c | 59 ++++++++++---------- dragonflybsd/DragonFlyBSDProcess.c | 6 ++- freebsd/FreeBSDProcess.c | 8 +-- linux/LinuxProcess.c | 86 +++++++++++++++++------------- openbsd/OpenBSDProcess.c | 2 + solaris/SolarisProcess.c | 18 ++++--- 7 files changed, 111 insertions(+), 84 deletions(-) diff --git a/Macros.h b/Macros.h index 1064dadb..64aaefa5 100644 --- a/Macros.h +++ b/Macros.h @@ -4,19 +4,27 @@ #include // IWYU pragma: keep #ifndef MINIMUM -#define MINIMUM(a, b) ((a) < (b) ? (a) : (b)) +#define MINIMUM(a, b) ((a) < (b) ? (a) : (b)) #endif #ifndef MAXIMUM -#define MAXIMUM(a, b) ((a) > (b) ? (a) : (b)) +#define MAXIMUM(a, b) ((a) > (b) ? (a) : (b)) #endif #ifndef CLAMP -#define CLAMP(x, low, high) (assert((low) <= (high)), ((x) > (high)) ? (high) : MAXIMUM(x, low)) +#define CLAMP(x, low, high) (assert((low) <= (high)), ((x) > (high)) ? (high) : MAXIMUM(x, low)) #endif #ifndef ARRAYSIZE -#define ARRAYSIZE(x) (sizeof(x) / sizeof((x)[0])) +#define ARRAYSIZE(x) (sizeof(x) / sizeof((x)[0])) +#endif + +#ifndef SPACESHIP_NUMBER +#define SPACESHIP_NUMBER(a, b) (((a) > (b)) - ((a) < (b))) +#endif + +#ifndef SPACESHIP_NULLSTR +#define SPACESHIP_NULLSTR(a, b) strcmp((a) ? (a) : "", (b) ? (b) : "") #endif #ifdef __GNUC__ // defined by GCC and Clang diff --git a/Process.c b/Process.c index 72360e39..427b5f62 100644 --- a/Process.c +++ b/Process.c @@ -460,6 +460,8 @@ long Process_pidCompare(const void* v1, const void* v2) { long Process_compare(const void* v1, const void* v2) { const Process *p1, *p2; const Settings *settings = ((const Process*)v1)->settings; + int r; + if (settings->direction == 1) { p1 = (const Process*)v1; p2 = (const Process*)v2; @@ -467,59 +469,56 @@ long Process_compare(const void* v1, const void* v2) { p2 = (const Process*)v1; p1 = (const Process*)v2; } + switch (settings->sortKey) { case PERCENT_CPU: - return (p2->percent_cpu > p1->percent_cpu ? 1 : -1); + return SPACESHIP_NUMBER(p2->percent_cpu, p1->percent_cpu); case PERCENT_MEM: - return (p2->m_resident - p1->m_resident); + return SPACESHIP_NUMBER(p2->m_resident, p1->m_resident); case COMM: - return strcmp(p1->comm, p2->comm); + return SPACESHIP_NULLSTR(p1->comm, p2->comm); case MAJFLT: - return (p2->majflt - p1->majflt); + return SPACESHIP_NUMBER(p2->majflt, p1->majflt); case MINFLT: - return (p2->minflt - p1->minflt); + return SPACESHIP_NUMBER(p2->minflt, p1->minflt); case M_RESIDENT: - return (p2->m_resident - p1->m_resident); + return SPACESHIP_NUMBER(p2->m_resident, p1->m_resident); case M_SIZE: - return (p2->m_size - p1->m_size); + return SPACESHIP_NUMBER(p2->m_size, p1->m_size); case NICE: - return (p1->nice - p2->nice); + return SPACESHIP_NUMBER(p1->nice, p2->nice); case NLWP: - return (p1->nlwp - p2->nlwp); + return SPACESHIP_NUMBER(p1->nlwp, p2->nlwp); case PGRP: - return (p1->pgrp - p2->pgrp); + return SPACESHIP_NUMBER(p1->pgrp, p2->pgrp); case PID: - return (p1->pid - p2->pid); + return SPACESHIP_NUMBER(p1->pid, p2->pid); case PPID: - return (p1->ppid - p2->ppid); + return SPACESHIP_NUMBER(p1->ppid, p2->ppid); case PRIORITY: - return (p1->priority - p2->priority); + return SPACESHIP_NUMBER(p1->priority, p2->priority); case PROCESSOR: - return (p1->processor - p2->processor); + return SPACESHIP_NUMBER(p1->processor, p2->processor); case SESSION: - return (p1->session - p2->session); - case STARTTIME: { - if (p1->starttime_ctime == p2->starttime_ctime) { - return (p1->pid - p2->pid); - } else { - return (p1->starttime_ctime - p2->starttime_ctime); - } - } + return SPACESHIP_NUMBER(p1->session, p2->session); + case STARTTIME: + r = SPACESHIP_NUMBER(p1->starttime_ctime, p2->starttime_ctime); + return r != 0 ? r : SPACESHIP_NUMBER(p1->pid, p2->pid); case STATE: - return (Process_sortState(p1->state) - Process_sortState(p2->state)); + return SPACESHIP_NUMBER(Process_sortState(p1->state), Process_sortState(p2->state)); case ST_UID: - return (p1->st_uid - p2->st_uid); + return SPACESHIP_NUMBER(p1->st_uid, p2->st_uid); case TIME: - return ((p2->time) - (p1->time)); + return SPACESHIP_NUMBER(p2->time, p1->time); case TGID: - return (p1->tgid - p2->tgid); + return SPACESHIP_NUMBER(p1->tgid, p2->tgid); case TPGID: - return (p1->tpgid - p2->tpgid); + return SPACESHIP_NUMBER(p1->tpgid, p2->tpgid); case TTY_NR: - return (p1->tty_nr - p2->tty_nr); + return SPACESHIP_NUMBER(p1->tty_nr, p2->tty_nr); case USER: - return strcmp(p1->user ? p1->user : "", p2->user ? p2->user : ""); + return SPACESHIP_NULLSTR(p1->user, p2->user); default: - return (p1->pid - p2->pid); + return SPACESHIP_NUMBER(p1->pid, p2->pid); } } diff --git a/dragonflybsd/DragonFlyBSDProcess.c b/dragonflybsd/DragonFlyBSDProcess.c index bd209327..55e4fbac 100644 --- a/dragonflybsd/DragonFlyBSDProcess.c +++ b/dragonflybsd/DragonFlyBSDProcess.c @@ -110,6 +110,7 @@ void DragonFlyBSDProcess_writeField(const Process* this, RichString* str, Proces long DragonFlyBSDProcess_compare(const void* v1, const void* v2) { const DragonFlyBSDProcess *p1, *p2; const Settings *settings = ((const Process*)v1)->settings; + if (settings->direction == 1) { p1 = (const DragonFlyBSDProcess*)v1; p2 = (const DragonFlyBSDProcess*)v2; @@ -117,12 +118,13 @@ long DragonFlyBSDProcess_compare(const void* v1, const void* v2) { p2 = (const DragonFlyBSDProcess*)v1; p1 = (const DragonFlyBSDProcess*)v2; } + switch ((int) settings->sortKey) { // add Platform-specific fields here case JID: - return (p1->jid - p2->jid); + return SPACESHIP_NUMBER(p1->jid, p2->jid); case JAIL: - return strcmp(p1->jname ? p1->jname : "", p2->jname ? p2->jname : ""); + return SPACESHIP_NULLSTR(p1->jname, p2->jname); default: return Process_compare(v1, v2); } diff --git a/freebsd/FreeBSDProcess.c b/freebsd/FreeBSDProcess.c index baad5d13..695594e8 100644 --- a/freebsd/FreeBSDProcess.c +++ b/freebsd/FreeBSDProcess.c @@ -111,6 +111,7 @@ static void FreeBSDProcess_writeField(const Process* this, RichString* str, Proc static long FreeBSDProcess_compare(const void* v1, const void* v2) { const FreeBSDProcess *p1, *p2; const Settings *settings = ((const Process*)v1)->settings; + if (settings->direction == 1) { p1 = (const FreeBSDProcess*)v1; p2 = (const FreeBSDProcess*)v2; @@ -118,14 +119,15 @@ static long FreeBSDProcess_compare(const void* v1, const void* v2) { p2 = (const FreeBSDProcess*)v1; p1 = (const FreeBSDProcess*)v2; } + switch ((int) settings->sortKey) { // add FreeBSD-specific fields here case JID: - return (p1->jid - p2->jid); + return SPACESHIP_NUMBER(p1->jid, p2->jid); case JAIL: - return strcmp(p1->jname ? p1->jname : "", p2->jname ? p2->jname : ""); + return SPACESHIP_NULLSTR(p1->jname, p2->jname); case TTY_NR: - return strcmp(p1->ttyPath ? p1->ttyPath : "", p2->ttyPath ? p2->ttyPath : ""); + return SPACESHIP_NULLSTR(p1->ttyPath, p2->ttyPath); default: return Process_compare(v1, v2); } diff --git a/linux/LinuxProcess.c b/linux/LinuxProcess.c index b1bd24d5..cd8db3d0 100644 --- a/linux/LinuxProcess.c +++ b/linux/LinuxProcess.c @@ -307,6 +307,7 @@ void LinuxProcess_writeField(const Process* this, RichString* str, ProcessField long LinuxProcess_compare(const void* v1, const void* v2) { const LinuxProcess *p1, *p2; const Settings *settings = ((const Process*)v1)->settings; + if (settings->direction == 1) { p1 = (const LinuxProcess*)v1; p2 = (const LinuxProcess*)v2; @@ -314,76 +315,87 @@ long LinuxProcess_compare(const void* v1, const void* v2) { p2 = (const LinuxProcess*)v1; p1 = (const LinuxProcess*)v2; } - long long diff; + switch ((int)settings->sortKey) { case M_DRS: - return (p2->m_drs - p1->m_drs); + return SPACESHIP_NUMBER(p2->m_drs, p1->m_drs); case M_DT: - return (p2->m_dt - p1->m_dt); + return SPACESHIP_NUMBER(p2->m_dt, p1->m_dt); case M_LRS: - return (p2->m_lrs - p1->m_lrs); + return SPACESHIP_NUMBER(p2->m_lrs, p1->m_lrs); case M_TRS: - return (p2->m_trs - p1->m_trs); + return SPACESHIP_NUMBER(p2->m_trs, p1->m_trs); case M_SHARE: - return (p2->m_share - p1->m_share); + return SPACESHIP_NUMBER(p2->m_share, p1->m_share); case M_PSS: - return (p2->m_pss - p1->m_pss); + return SPACESHIP_NUMBER(p2->m_pss, p1->m_pss); case M_SWAP: - return (p2->m_swap - p1->m_swap); + return SPACESHIP_NUMBER(p2->m_swap, p1->m_swap); case M_PSSWP: - return (p2->m_psswp - p1->m_psswp); - case UTIME: diff = p2->utime - p1->utime; goto test_diff; - case CUTIME: diff = p2->cutime - p1->cutime; goto test_diff; - case STIME: diff = p2->stime - p1->stime; goto test_diff; - case CSTIME: diff = p2->cstime - p1->cstime; goto test_diff; + return SPACESHIP_NUMBER(p2->m_psswp, p1->m_psswp); + case UTIME: + return SPACESHIP_NUMBER(p2->utime, p1->utime); + case CUTIME: + return SPACESHIP_NUMBER(p2->cutime, p1->cutime); + case STIME: + return SPACESHIP_NUMBER(p2->stime, p1->stime); + case CSTIME: + return SPACESHIP_NUMBER(p2->cstime, p1->cstime); #ifdef HAVE_TASKSTATS - case RCHAR: diff = p2->io_rchar - p1->io_rchar; goto test_diff; - case WCHAR: diff = p2->io_wchar - p1->io_wchar; goto test_diff; - case SYSCR: diff = p2->io_syscr - p1->io_syscr; goto test_diff; - case SYSCW: diff = p2->io_syscw - p1->io_syscw; goto test_diff; - case RBYTES: diff = p2->io_read_bytes - p1->io_read_bytes; goto test_diff; - case WBYTES: diff = p2->io_write_bytes - p1->io_write_bytes; goto test_diff; - case CNCLWB: diff = p2->io_cancelled_write_bytes - p1->io_cancelled_write_bytes; goto test_diff; - case IO_READ_RATE: diff = p2->io_rate_read_bps - p1->io_rate_read_bps; goto test_diff; - case IO_WRITE_RATE: diff = p2->io_rate_write_bps - p1->io_rate_write_bps; goto test_diff; - case IO_RATE: diff = (p2->io_rate_read_bps + p2->io_rate_write_bps) - (p1->io_rate_read_bps + p1->io_rate_write_bps); goto test_diff; + case RCHAR: + return SPACESHIP_NUMBER(p2->io_rchar, p1->io_rchar); + case WCHAR: + return SPACESHIP_NUMBER(p2->io_wchar, p1->io_wchar); + case SYSCR: + return SPACESHIP_NUMBER(p2->io_syscr, p1->io_syscr); + case SYSCW: + return SPACESHIP_NUMBER(p2->io_syscw, p1->io_syscw); + case RBYTES: + return SPACESHIP_NUMBER(p2->io_read_bytes, p1->io_read_bytes); + case WBYTES: + return SPACESHIP_NUMBER(p2->io_write_bytes, p1->io_write_bytes); + case CNCLWB: + return SPACESHIP_NUMBER(p2->io_cancelled_write_bytes, p1->io_cancelled_write_bytes); + case IO_READ_RATE: + return SPACESHIP_NUMBER(p2->io_rate_read_bps, p1->io_rate_read_bps); + case IO_WRITE_RATE: + return SPACESHIP_NUMBER(p2->io_rate_write_bps, p1->io_rate_write_bps); + case IO_RATE: + return SPACESHIP_NUMBER(p2->io_rate_read_bps + p2->io_rate_write_bps, p1->io_rate_read_bps + p1->io_rate_write_bps); #endif #ifdef HAVE_OPENVZ case CTID: - return strcmp(p1->ctid ? p1->ctid : "", p2->ctid ? p2->ctid : ""); + return SPACESHIP_NULLSTR(p1->ctid, p2->ctid); case VPID: - return (p2->vpid - p1->vpid); + return SPACESHIP_NUMBER(p2->vpid, p1->vpid); #endif #ifdef HAVE_VSERVER case VXID: - return (p2->vxid - p1->vxid); + return SPACESHIP_NUMBER(p2->vxid, p1->vxid); #endif #ifdef HAVE_CGROUP case CGROUP: - return strcmp(p1->cgroup ? p1->cgroup : "", p2->cgroup ? p2->cgroup : ""); + return SPACESHIP_NULLSTR(p1->cgroup, p2->cgroup); #endif case OOM: - return ((int)p2->oom - (int)p1->oom); + return SPACESHIP_NUMBER(p2->oom, p1->oom); #ifdef HAVE_DELAYACCT case PERCENT_CPU_DELAY: - return (p2->cpu_delay_percent > p1->cpu_delay_percent ? 1 : -1); + return SPACESHIP_NUMBER(p2->cpu_delay_percent, p1->cpu_delay_percent); case PERCENT_IO_DELAY: - return (p2->blkio_delay_percent > p1->blkio_delay_percent ? 1 : -1); + return SPACESHIP_NUMBER(p2->blkio_delay_percent, p1->blkio_delay_percent); case PERCENT_SWAP_DELAY: - return (p2->swapin_delay_percent > p1->swapin_delay_percent ? 1 : -1); + return SPACESHIP_NUMBER(p2->swapin_delay_percent, p1->swapin_delay_percent); #endif case IO_PRIORITY: - return LinuxProcess_effectiveIOPriority(p1) - LinuxProcess_effectiveIOPriority(p2); + return SPACESHIP_NUMBER(LinuxProcess_effectiveIOPriority(p1), LinuxProcess_effectiveIOPriority(p2)); case CTXT: - return ((long)p2->ctxt_diff - (long)p1->ctxt_diff); + return SPACESHIP_NUMBER(p2->ctxt_diff, p1->ctxt_diff); case SECATTR: - return strcmp(p1->secattr ? p1->secattr : "", p2->secattr ? p2->secattr : ""); + return SPACESHIP_NULLSTR(p1->secattr, p2->secattr); default: return Process_compare(v1, v2); } - -test_diff: - return (diff > 0) ? 1 : (diff < 0 ? -1 : 0); } bool Process_isThread(const Process* this) { diff --git a/openbsd/OpenBSDProcess.c b/openbsd/OpenBSDProcess.c index 5eb61b16..1013d3b4 100644 --- a/openbsd/OpenBSDProcess.c +++ b/openbsd/OpenBSDProcess.c @@ -220,6 +220,7 @@ void OpenBSDProcess_writeField(const Process* this, RichString* str, ProcessFiel long OpenBSDProcess_compare(const void* v1, const void* v2) { const OpenBSDProcess *p1, *p2; const Settings *settings = ((const Process*)v1)->settings; + if (settings->direction == 1) { p1 = (const OpenBSDProcess*)v1; p2 = (const OpenBSDProcess*)v2; @@ -227,6 +228,7 @@ long OpenBSDProcess_compare(const void* v1, const void* v2) { p2 = (const OpenBSDProcess*)v1; p1 = (const OpenBSDProcess*)v2; } + switch (settings->sortKey) { // add OpenBSD-specific fields here default: diff --git a/solaris/SolarisProcess.c b/solaris/SolarisProcess.c index 9c6c7d77..7d833fe9 100644 --- a/solaris/SolarisProcess.c +++ b/solaris/SolarisProcess.c @@ -119,6 +119,7 @@ void SolarisProcess_writeField(const Process* this, RichString* str, ProcessFiel long SolarisProcess_compare(const void* v1, const void* v2) { const SolarisProcess *p1, *p2; const Settings* settings = ((const Process*)v1)->settings; + if (settings->direction == 1) { p1 = (const SolarisProcess*)v1; p2 = (const SolarisProcess*)v2; @@ -126,25 +127,26 @@ long SolarisProcess_compare(const void* v1, const void* v2) { p2 = (const SolarisProcess*)v1; p1 = (const SolarisProcess*)v2; } + switch ((int) settings->sortKey) { case ZONEID: - return (p1->zoneid - p2->zoneid); + return SPACESHIP_NUMBER(p1->zoneid, p2->zoneid); case PROJID: - return (p1->projid - p2->projid); + return SPACESHIP_NUMBER(p1->projid, p2->projid); case TASKID: - return (p1->taskid - p2->taskid); + return SPACESHIP_NUMBER(p1->taskid, p2->taskid); case POOLID: - return (p1->poolid - p2->poolid); + return SPACESHIP_NUMBER(p1->poolid, p2->poolid); case CONTID: - return (p1->contid - p2->contid); + return SPACESHIP_NUMBER(p1->contid, p2->contid); case ZONE: return strcmp(p1->zname ? p1->zname : "global", p2->zname ? p2->zname : "global"); case PID: - return (p1->realpid - p2->realpid); + return SPACESHIP_NUMBER(p1->realpid, p2->realpid); case PPID: - return (p1->realppid - p2->realppid); + return SPACESHIP_NUMBER(p1->realppid, p2->realppid); case LWPID: - return (p1->lwpid - p2->lwpid); + return SPACESHIP_NUMBER(p1->lwpid, p2->lwpid); default: return Process_compare(v1, v2); }