From 06f1b34553b868aced6fe51bd0493b83c7fdfea6 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Fri, 20 Nov 2020 14:22:42 -0600 Subject: [PATCH] Correct reporting of setpgid (parent vs child) Previously, it always said "own process" (e.g. child error). --- src/exec.cpp | 4 ++-- src/postfork.cpp | 8 +++++--- src/postfork.h | 3 ++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index 0437fb1b8..c31bd4834 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -337,7 +337,7 @@ static bool fork_child_for_process(const std::shared_ptr &job, process_t // The child attempts to join the pgroup. if (int err = execute_setpgid(p->pid, pgid, false /* not parent */)) { - report_setpgid_error(err, pgid, job.get(), p); + report_setpgid_error(err, false /* is_parent */, pgid, job.get(), p); } child_setup_process(claim_tty ? pgid : INVALID_PID, fish_pgrp, *job, true, dup2s); child_action(); @@ -362,7 +362,7 @@ static bool fork_child_for_process(const std::shared_ptr &job, process_t // The parent attempts to send the child to its pgroup. // EACCESS is an expected benign error as the child may have called exec(). if (int err = execute_setpgid(p->pid, pgid, true /* is parent */)) { - if (err != EACCES) report_setpgid_error(err, pgid, job.get(), p); + if (err != EACCES) report_setpgid_error(err, true /* is_parent */, pgid, job.get(), p); } terminal_maybe_give_to_job_group(job->group.get(), false); return true; diff --git a/src/postfork.cpp b/src/postfork.cpp index ec82b6842..4c2169377 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -42,7 +42,8 @@ static char *get_interpreter(const char *command, char *buffer, size_t buff_size); /// Report the error code \p err for a failed setpgid call. -void report_setpgid_error(int err, pid_t desired_pgid, const job_t *j, const process_t *p) { +void report_setpgid_error(int err, bool is_parent, pid_t desired_pgid, const job_t *j, + const process_t *p) { char pid_buff[128]; char job_id_buff[128]; char getpgid_buff[128]; @@ -57,8 +58,9 @@ void report_setpgid_error(int err, pid_t desired_pgid, const job_t *j, const pro narrow_string_safe(argv0, p->argv0()); narrow_string_safe(command, j->command_wcstr()); - debug_safe(1, "Could not send own process %s, '%s' in job %s, '%s' from group %s to group %s", - pid_buff, argv0, job_id_buff, command, getpgid_buff, job_pgid_buff); + debug_safe(1, "Could not send %s %s, '%s' in job %s, '%s' from group %s to group %s", + is_parent ? "child" : "self", pid_buff, argv0, job_id_buff, command, getpgid_buff, + job_pgid_buff); if (is_windows_subsystem_for_linux() && errno == EPERM) { debug_safe(1, diff --git a/src/postfork.h b/src/postfork.h index 97ddbf572..0baac984b 100644 --- a/src/postfork.h +++ b/src/postfork.h @@ -30,7 +30,8 @@ int execute_setpgid(pid_t pid, pid_t pgroup, bool is_parent); /// Report the error code \p err for a failed setpgid call. /// Note not all errors should be reported; in particular EACCESS is expected and benign in the /// parent only. -void report_setpgid_error(int err, pid_t desired_pgid, const job_t *j, const process_t *p); +void report_setpgid_error(int err, bool is_parent, pid_t desired_pgid, const job_t *j, + const process_t *p); /// Initialize a new child process. This should be called right away after forking in the child /// process. If job control is enabled for this job, the process is put in the process group of the