From 35c7070d3cf645dae3057ca68d458e8df3853fa3 Mon Sep 17 00:00:00 2001 From: Herman Semenoff Date: Tue, 10 Feb 2026 15:58:56 +0300 Subject: [PATCH] trace/openfiles screen: fix unnecessary copying parent memory page table posix_spawn() is a more lightweight and modern alternative to fork()/exec(), which are used inside popen(). The main advantage of posix_spawn is that it can create a new process without completely duplicating the address space of the parent process. References: - https://blog.famzah.net/2018/12/19/posix_spawn-performance-benchmarks-and-usage-examples/ - https://lobste.rs/s/smbsd5/fork_road - https://www.reddit.com/r/C_Programming/comments/1lvdhp2/fork_vs_posix_spawn/ --- OpenFilesScreen.c | 53 +++++++++++++++++++++-------------- TraceScreen.c | 70 ++++++++++++++++++++++++----------------------- 2 files changed, 69 insertions(+), 54 deletions(-) diff --git a/OpenFilesScreen.c b/OpenFilesScreen.c index d04cce76e..04e54095f 100644 --- a/OpenFilesScreen.c +++ b/OpenFilesScreen.c @@ -12,6 +12,7 @@ in the source distribution for its full text. #include #include #include +#include #include #include #include @@ -27,6 +28,9 @@ in the source distribution for its full text. #include "XUtils.h" +// Helper to create a mutable string for argv arrays +#define MUTABLE_STR(s) (char[]){s} + // cf. getIndexForType; must be larger than the maximum value returned. #define LSOF_DATACOL_COUNT 8 @@ -106,32 +110,41 @@ static OpenFiles_ProcessData* OpenFilesScreen_getProcessData(pid_t pid) { return pdata; } - pid_t child = fork(); - if (child == -1) { + pid_t child; + posix_spawnattr_t attr; + posix_spawnattr_init(&attr); + posix_spawn_file_actions_t fa; + posix_spawn_file_actions_init(&fa); + posix_spawn_file_actions_addclose(&fa, fdpair[0]); + posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDOUT_FILENO); + posix_spawn_file_actions_addclose(&fa, fdpair[1]); + posix_spawn_file_actions_addopen(&fa, STDERR_FILENO, "/dev/null", O_WRONLY | O_NOCTTY, 0); + + char buffer[32] = {0}; + xSnprintf(buffer, sizeof(buffer), "%d", pid); + + char* const args[] = { + MUTABLE_STR("lsof"), + MUTABLE_STR("-P"), + MUTABLE_STR("-o"), + MUTABLE_STR("-p"), + buffer, + MUTABLE_STR("-F"), + NULL + }; + + int spawn_ret = posix_spawnp(&child, "lsof", &fa, &attr, args, NULL); + + posix_spawnattr_destroy(&attr); + posix_spawn_file_actions_destroy(&fa); + + if (spawn_ret != 0) { close(fdpair[1]); close(fdpair[0]); pdata->error = 1; return pdata; } - if (child == 0) { - close(fdpair[0]); - dup2(fdpair[1], STDOUT_FILENO); - close(fdpair[1]); - int fdnull = open("/dev/null", O_WRONLY); - if (fdnull < 0) { - exit(1); - } - - dup2(fdnull, STDERR_FILENO); - close(fdnull); - char buffer[32] = {0}; - xSnprintf(buffer, sizeof(buffer), "%d", pid); - // Use of NULL in variadic functions must have a pointer cast. - // The NULL constant is not required by standard to have a pointer type. - execlp("lsof", "lsof", "-P", "-o", "-p", buffer, "-F", (char*)NULL); - exit(127); - } close(fdpair[1]); OpenFiles_Data* item = &(pdata->data); diff --git a/TraceScreen.c b/TraceScreen.c index 834b7667e..79eb0e9f6 100644 --- a/TraceScreen.c +++ b/TraceScreen.c @@ -13,6 +13,7 @@ in the source distribution for its full text. #include #include #include +#include #include #include #include @@ -28,6 +29,9 @@ in the source distribution for its full text. #include "XUtils.h" +// Helper to create a mutable string for argv arrays +#define MUTABLE_STR(s) (char[]){s} + static const char* const TraceScreenFunctions[] = {"Search ", "Filter ", "AutoScroll ", "Stop Tracing ", "Done ", NULL}; static const char* const TraceScreenKeys[] = {"F3", "F4", "F8", "F9", "Esc"}; @@ -78,45 +82,43 @@ bool TraceScreen_forkTracer(TraceScreen* this) { if (fcntl(fdpair[1], F_SETFL, O_NONBLOCK) < 0) goto err; - pid_t child = fork(); - if (child == -1) + pid_t child; + posix_spawnattr_t attr; + posix_spawnattr_init(&attr); + posix_spawn_file_actions_t fa; + posix_spawn_file_actions_init(&fa); + posix_spawn_file_actions_addclose(&fa, fdpair[0]); + posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDOUT_FILENO); + posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDERR_FILENO); + posix_spawn_file_actions_addclose(&fa, fdpair[1]); + + char buffer[32] = {0}; + xSnprintf(buffer, sizeof(buffer), "%d", Process_getPid(this->super.process)); + + char* const* args; +#if defined(HTOP_FREEBSD) || defined(HTOP_OPENBSD) || defined(HTOP_NETBSD) || defined(HTOP_DRAGONFLYBSD) || defined(HTOP_SOLARIS) + args = (char* const[]){MUTABLE_STR("truss"), MUTABLE_STR("-s"), MUTABLE_STR("512"), MUTABLE_STR("-p"), buffer, NULL}; +#elif defined(HTOP_LINUX) + args = (char* const[]){MUTABLE_STR("strace"), MUTABLE_STR("-T"), MUTABLE_STR("-tt"), MUTABLE_STR("-s"), MUTABLE_STR("512"), MUTABLE_STR("-p"), buffer, NULL}; +#else + args = NULL; +#endif + + int spawn_ret = args ? posix_spawnp(&child, args[0], &fa, &attr, args, NULL) : -1; + + posix_spawnattr_destroy(&attr); + posix_spawn_file_actions_destroy(&fa); + + if (spawn_ret != 0) { goto err; - - if (child == 0) { - close(fdpair[0]); - - dup2(fdpair[1], STDOUT_FILENO); - dup2(fdpair[1], STDERR_FILENO); - close(fdpair[1]); - - char buffer[32] = {0}; - xSnprintf(buffer, sizeof(buffer), "%d", Process_getPid(this->super.process)); - - #if defined(HTOP_FREEBSD) || defined(HTOP_OPENBSD) || defined(HTOP_NETBSD) || defined(HTOP_DRAGONFLYBSD) || defined(HTOP_SOLARIS) - // Use of NULL in variadic functions must have a pointer cast. - // The NULL constant is not required by standard to have a pointer type. - execlp("truss", "truss", "-s", "512", "-p", buffer, (void*)NULL); - - // Should never reach here, unless execlp fails ... - const char* message = "Could not execute 'truss'. Please make sure it is available in your $PATH."; - (void)! write(STDERR_FILENO, message, strlen(message)); - #elif defined(HTOP_LINUX) - execlp("strace", "strace", "-T", "-tt", "-s", "512", "-p", buffer, (void*)NULL); - - // Should never reach here, unless execlp fails ... - const char* message = "Could not execute 'strace'. Please make sure it is available in your $PATH."; - (void)! write(STDERR_FILENO, message, strlen(message)); - #else // HTOP_DARWIN, HTOP_PCP == HTOP_UNSUPPORTED - const char* message = "Tracing unavailable on not supported system."; - (void)! write(STDERR_FILENO, message, strlen(message)); - #endif - - exit(127); } FILE* fp = fdopen(fdpair[0], "r"); - if (!fp) + if (!fp) { + kill(child, SIGTERM); + waitpid(child, NULL, 0); goto err; + } close(fdpair[1]);