osc-context: several follow-ups (systemd#36579)

src/core/main.c

@@ -1514,6 +1514,38 @@ static int write_container_id(void) {
         return 1;
 }
 
+static int write_boot_or_shutdown_osc(const char *type) {
+        int r;
+
+        assert(STRPTR_IN_SET(type, "boot", "shutdown"));
+
+        if (getenv_terminal_is_dumb())
+                return 0;
+
+        _cleanup_close_ int fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC);
+        if (fd < 0)
+                return log_debug_errno(fd, "Failed to open /dev/console to print %s OSC, ignoring: %m", type);
+
+        _cleanup_free_ char *seq = NULL;
+        if (streq(type, "boot"))
+                r = osc_context_open_boot(&seq);
+        else
+                r = osc_context_close(SD_ID128_ALLF, &seq);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to acquire %s OSC sequence, ignoring: %m", type);
+
+        r = loop_write(fd, seq, SIZE_MAX);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to write %s OSC sequence, ignoring: %m", type);
+
+        if (DEBUG_LOGGING) {
+                _cleanup_free_ char *h = cescape(seq);
+                log_debug("OSC sequence for %s successfully written: %s", type, strna(h));
+        }
+
+        return 0;
+}
+
 static int bump_unix_max_dgram_qlen(void) {
         _cleanup_free_ char *qlen = NULL;
         unsigned long v;
@@ -1707,6 +1739,8 @@ static int become_shutdown(int objective, int retval) {
         if (detect_container() <= 0)
                 (void) cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
 
+        (void) write_boot_or_shutdown_osc("shutdown");
+
         execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, env_block);
         return -errno;
 }
@@ -2385,38 +2419,6 @@ static void log_execution_mode(bool *ret_first_boot) {
         *ret_first_boot = first_boot;
 }
 
-static int write_boot_or_shutdown_osc(const char *type) {
-        int r;
-
-        assert(STRPTR_IN_SET(type, "boot", "shutdown"));
-
-        if (getenv_terminal_is_dumb())
-                return 0;
-
-        _cleanup_close_ int fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC);
-        if (fd < 0)
-                return log_debug_errno(fd, "Failed to open /dev/console to print %s OSC, ignoring: %m", type);
-
-        _cleanup_free_ char *seq = NULL;
-        if (streq(type, "boot"))
-                r = osc_context_open_boot(&seq);
-        else
-                r = osc_context_close(SD_ID128_ALLF, &seq);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to acquire %s OSC sequence, ignoring: %m", type);
-
-        r = loop_write(fd, seq, SIZE_MAX);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to write %s OSC sequence, ignoring: %m", type);
-
-        if (DEBUG_LOGGING) {
-                _cleanup_free_ char *h = cescape(seq);
-                log_debug("OSC sequence for %s successfully written: %s", type, strna(h));
-        }
-
-        return 0;
-}
-
 static int initialize_runtime(
                 bool skip_setup,
                 bool first_boot,
@@ -3459,8 +3461,6 @@ int main(int argc, char *argv[]) {
         }
 #endif
 
-        (void) write_boot_or_shutdown_osc("shutdown");
-
         if (r < 0)
                 (void) sd_notifyf(/* unset_environment= */ false,
                                   "ERRNO=%i", -r);

src/login/pam_systemd.c

@@ -1604,7 +1604,7 @@ static int open_osc_context(pam_handle_t *handle, const char *session_type, User
          * high-level for us, as it suffixes the output with a newline, expecting a full blown text message
          * as prompt string, not just an ANSI sequence. Note that PAM's conv_misc() actually goes to stdout
          * anyway, hence let's do so here too, but only after careful validation. */
-        if (!isatty(STDOUT_FILENO))
+        if (!isatty_safe(STDOUT_FILENO))
                 return PAM_SUCCESS;
 
         /* Keep a reference to the TTY we are operating on, so that we can issue the OSC close sequence also
@@ -1688,7 +1688,7 @@ static int close_osc_context(pam_handle_t *handle) {
         fd = fd_move_above_stdio(fd);
 
         /* Safety check, let's verify this is a valid TTY we just opened */
-        if (!isatty(fd))
+        if (!isatty_safe(fd))
                 return PAM_SUCCESS;
 
         _cleanup_free_ char *osc = NULL;

src/run/run.c

@@ -1233,8 +1233,7 @@ static int transient_kill_set_properties(sd_bus_message *m) {
 }
 
 static int transient_service_set_properties(sd_bus_message *m, const char *pty_path, int pty_fd) {
-        int send_term = false; /* tri-state */
-        int r;
+        int r, send_term; /* tri-state */
 
         /* We disable environment expansion on the server side via ExecStartEx=:.
          * ExecStartEx was added relatively recently (v243), and some bugs were fixed only later.
@@ -1313,15 +1312,16 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p
                         return bus_log_create_error(r);
 
                 send_term = -1;
-        }
+        } else
+                send_term = false;
 
         if (send_term != 0) {
                 const char *e;
 
-                /* Propagate $TERM only if we are actually connected to a TTY  */
+                /* Propagate $TERM only if we are actually connected to a TTY */
                 if (isatty_safe(STDIN_FILENO) || isatty_safe(STDOUT_FILENO) || isatty_safe(STDERR_FILENO)) {
                         e = getenv("TERM");
-                        send_term = true;
+                        send_term = !!e;
                 } else
                         /* If we are not connected to any TTY ourselves, then send TERM=dumb, but only if we
                          * really need to (because we actually allocated a TTY for the service) */
@@ -2205,10 +2205,10 @@ static int start_transient_service(sd_bus *bus) {
 
                 _cleanup_(osc_context_closep) sd_id128_t osc_context_id = SD_ID128_NULL;
                 if (pty_fd >= 0) {
-                        if (!terminal_is_dumb() && arg_exec_user) {
+                        if (arg_exec_user && !terminal_is_dumb()) {
                                 r = osc_context_open_chpriv(arg_exec_user, /* ret_seq= */ NULL, &osc_context_id);
                                 if (r < 0)
-                                        return r;
+                                        return log_error_errno(r, "Failed to set OSC context: %m");
                         }
 
                         (void) sd_event_set_signal_exit(c.event, true);

src/shared/osc-context.c

@@ -141,15 +141,13 @@ static int osc_context_intro(char **ret_seq, sd_id128_t *ret_context_id) {
         } else
                 osc_context_default_id(&id);
 
-        _cleanup_free_ char *seq = NULL;
-        r = osc_context_intro_raw(id, &seq);
+        r = osc_context_intro_raw(id, ret_seq);
         if (r < 0)
                 return r;
 
         if (ret_context_id)
                 *ret_context_id = id;
 
-        *ret_seq = TAKE_PTR(seq);
         return 0;
 }