devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 6 Actions Packages Projects 1 Releases Wiki Activity

fix(editor): follow live terminal size in external editor #153

Open
jabber.developer2 wants to merge 1 commits from fix/editor-terminal-size into master
pull from: fix/editor-terminal-size
merge into: devs:master
Conversation 1 Commits 1 Files Changed 1 +22
1 changed files with 22 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/tools/editor.c
View File

@@ -25,6 +25,8 @@
#include "ui/ui.h"
#include "xmpp/xmpp.h"
extern char** environ;
typedef struct EditorContext
{
gchar* filename;
@@ -140,6 +142,22 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
GSource* sigchld_warmup = g_child_watch_source_new(getpid());
g_source_unref(sigchld_warmup);
// Build the editor's env without LINES/COLUMNS pre-fork, so the child only
// reassigns environ instead of calling unsetenv() between fork and exec.
// The editor's (n)curses then reads the live window via ioctl(TIOCGWINSZ).
gsize env_len = 0;
while (environ[env_len]) {
env_len++;
}
gchar** editor_env = g_new0(gchar*, env_len + 1);
gsize env_kept = 0;
for (gsize i = 0; i < env_len; i++) {
if (g_str_has_prefix(environ[i], "LINES=") || g_str_has_prefix(environ[i], "COLUMNS=")) {
continue;
}
editor_env[env_kept++] = environ[i];
}
pid_t pid = fork();
if (pid == -1) {
log_error("[Editor] Failed to fork: %s", strerror(errno));
@@ -148,12 +166,15 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
ui_resize();
cons_show_error("Failed to start editor: %s", strerror(errno));
g_strfreev(editor_argv);
g_free(editor_env); // array only; strings are borrowed from environ
jabber.developer commented 2026-07-06 09:51:41 +00:00
Outdated
Review
Copy Link

Could it get freed prior to COW completion on fork?

Could it get freed prior to COW completion on fork?
jabber.developer2 commented 2026-07-13 12:33:36 +00:00
Outdated
Review
Copy Link

Could it get freed prior to COW completion on fork?

Should be safe. After fork() the child has its own address space, so the
array (and the borrowed string pointers in it) is the child's private
copy-on-write copy. g_free() runs only in the parent — that write faults
in a private page for the parent and leaves the child's copy intact. The
child reads environ only up to execvp(), which then replaces the whole
address space (the kernel snapshots argv/envp first), so it never touches
anything the parent freed, regardless of scheduling order. Only the array
is freed, never the strings — they stay owned by the original environ.

"async-signal-safe" might cause wrong assumptions

Agreed, reworded. The intent wasn't to claim the pointer store itself
buys any guarantee, only that the child avoids unsetenv(), whose
allocator/locking work is the part that's unsafe to run after fork in a
multithreaded process. U

> Could it get freed prior to COW completion on fork? Should be safe. After fork() the child has its own address space, so the array (and the borrowed string pointers in it) is the child's private copy-on-write copy. g_free() runs only in the parent — that write faults in a private page for the parent and leaves the child's copy intact. The child reads environ only up to execvp(), which then replaces the whole address space (the kernel snapshots argv/envp first), so it never touches anything the parent freed, regardless of scheduling order. Only the array is freed, never the strings — they stay owned by the original environ. > "async-signal-safe" might cause wrong assumptions Agreed, reworded. The intent wasn't to claim the pointer store itself buys any guarantee, only that the child avoids unsetenv(), whose allocator/locking work is the part that's unsafe to run after fork in a multithreaded process. U
g_free(ctx->filename);
g_free(ctx);
return TRUE;
} else if (pid == 0) {
// Child process: Inherits TTY from parent
environ = editor_env; // live TIOCGWINSZ size, not the inherited LINES/COLUMNS
// SIGTSTP=SIG_DFL lets vim's :stop / Ctrl-Z work; profanity catches
// the STOPPED state via editor_check_stopped() and drops to the shell.
signal(SIGINT, SIG_DFL);
@@ -170,6 +191,7 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
editor_pid = pid;
g_child_watch_add((GPid)pid, _editor_exit_cb, ctx);
g_strfreev(editor_argv);
g_free(editor_env); // frees the parent's array only; the child keeps its own COW copy until execvp
return FALSE;
}