Terminal: Modernize

- Replace C with C++
- Use Core::System, Core::Account and Core::Process wrappers
- Remove DeprecatedString
- Remove invalid close() call to the pts file descriptor in the shell
  child (the fd cannot be valid there anymore since it's an
  automatically-closing fd, we just previously ignored this error)

Userland/Applications/Terminal/main.cpp

@@ -6,9 +6,11 @@
 
 #include <AK/FixedArray.h>
 #include <AK/QuickSort.h>
+#include <AK/TypedTransfer.h>
 #include <AK/URL.h>
 #include <LibConfig/Client.h>
 #include <LibConfig/Listener.h>
+#include <LibCore/Account.h>
 #include <LibCore/ArgsParser.h>
 #include <LibCore/Directory.h>
 #include <LibCore/System.h>
@@ -35,17 +37,7 @@
 #include <LibGfx/Palette.h>
 #include <LibMain/Main.h>
 #include <LibVT/TerminalWidget.h>
-#include <assert.h>
-#include <errno.h>
 #include <pty.h>
-#include <pwd.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/ioctl.h>
-#include <sys/wait.h>
-#include <unistd.h>
 
 class TerminalChangeListener : public Config::Listener {
 public:
@@ -111,45 +103,45 @@ private:
     VT::TerminalWidget& m_parent_terminal;
 };
 
-static void utmp_update(DeprecatedString const& tty, pid_t pid, bool create)
+static ErrorOr<void> utmp_update(StringView tty, pid_t pid, bool create)
 {
-    int utmpupdate_pid = fork();
-    if (utmpupdate_pid < 0) {
-        perror("fork");
-        return;
-    }
-    if (utmpupdate_pid == 0) {
-        // Be careful here! Because fork() only clones one thread it's
-        // possible that we deadlock on anything involving a mutex,
-        // including the heap! So resort to low-level APIs
-        char pid_str[32];
-        snprintf(pid_str, sizeof(pid_str), "%d", pid);
-        execl("/bin/utmpupdate", "/bin/utmpupdate", "-f", "Terminal", "-p", pid_str, (create ? "-c" : "-d"), tty.characters(), nullptr);
-    } else {
-    wait_again:
-        int status = 0;
-        if (waitpid(utmpupdate_pid, &status, 0) < 0) {
-            int err = errno;
-            if (err == EINTR)
-                goto wait_again;
-            perror("waitpid");
-            return;
+    auto pid_string = TRY(String::number(pid));
+    Array utmp_update_command {
+        "-f"sv,
+        "Terminal"sv,
+        "-p"sv,
+        pid_string.bytes_as_string_view(),
+        (create ? "-c"sv : "-d"sv),
+        tty,
+    };
+
+    auto utmpupdate_pid = TRY(Core::Process::spawn("/bin/utmpupdate"sv, utmp_update_command, {}, Core::Process::KeepAsChild::Yes));
+
+    Core::System::WaitPidResult status;
+    auto wait_successful = false;
+    while (!wait_successful) {
+        auto result = Core::System::waitpid(utmpupdate_pid, 0);
+        if (result.is_error() && result.error().code() != EINTR) {
+            return result.release_error();
+        } else if (!result.is_error()) {
+            status = result.release_value();
+            wait_successful = true;
         }
-        if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
-            dbgln("Terminal: utmpupdate exited with status {}", WEXITSTATUS(status));
-        else if (WIFSIGNALED(status))
-            dbgln("Terminal: utmpupdate exited due to unhandled signal {}", WTERMSIG(status));
     }
+
+    if (WIFEXITED(status.status) && WEXITSTATUS(status.status) != 0)
+        dbgln("Terminal: utmpupdate exited with status {}", WEXITSTATUS(status.status));
+    else if (WIFSIGNALED(status.status))
+        dbgln("Terminal: utmpupdate exited due to unhandled signal {}", WTERMSIG(status.status));
+
+    return {};
 }
 
-static ErrorOr<void> run_command(DeprecatedString command, bool keep_open)
+static ErrorOr<void> run_command(StringView command, bool keep_open)
 {
-    DeprecatedString shell = "/bin/Shell";
-    auto* pw = getpwuid(getuid());
-    if (pw && pw->pw_shell) {
-        shell = pw->pw_shell;
-    }
-    endpwent();
+    auto shell = TRY(String::from_deprecated_string(TRY(Core::Account::self(Core::Account::Read::PasswdOnly)).shell()));
+    if (shell.is_empty())
+        shell = TRY("/bin/Shell"_string);
 
     Vector<StringView> arguments;
     arguments.append(shell);
@@ -239,7 +231,10 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     TRY(Core::System::pledge("stdio tty rpath cpath wpath recvfd sendfd proc exec unix sigaction"));
 
     struct sigaction act;
-    memset(&act, 0, sizeof(act));
+    act.sa_mask = 0;
+    // Do not trust that both function pointers overlap.
+    act.sa_sigaction = nullptr;
+
     act.sa_flags = SA_NOCLDWAIT;
     act.sa_handler = SIG_IGN;
 
@@ -267,12 +262,11 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     int ptm_fd;
     pid_t shell_pid = forkpty(&ptm_fd, nullptr, nullptr, nullptr);
-    if (shell_pid < 0) {
-        perror("forkpty");
-        return 1;
-    }
+    if (shell_pid < 0)
+        return Error::from_errno(errno);
+
+    // We're the child process; run the startup command.
     if (shell_pid == 0) {
-        close(ptm_fd);
         if (!command_to_execute.is_empty())
             TRY(run_command(command_to_execute, keep_open));
         else
@@ -281,7 +275,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     }
 
     auto ptsname = TRY(Core::System::ptsname(ptm_fd));
-    utmp_update(ptsname, shell_pid, true);
+    TRY(utmp_update(ptsname, shell_pid, true));
 
     auto app_icon = GUI::Icon::default_icon("app-terminal"sv);
 
@@ -356,7 +350,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     auto shell_child_process_count = [&] {
         int background_process_count = 0;
-        Core::Directory::for_each_entry(DeprecatedString::formatted("/proc/{}/children", shell_pid), Core::DirIterator::Flags::SkipParentAndBaseDir, [&](auto&, auto&) {
+        Core::Directory::for_each_entry(String::formatted("/proc/{}/children", shell_pid).release_value_but_fixme_should_propagate_errors(), Core::DirIterator::Flags::SkipParentAndBaseDir, [&](auto&, auto&) {
             ++background_process_count;
             return IterationDecision::Continue;
         }).release_value_but_fixme_should_propagate_errors();
@@ -366,17 +360,17 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     auto check_terminal_quit = [&]() -> GUI::Dialog::ExecResult {
         if (!should_confirm_close)
             return GUI::MessageBox::ExecResult::OK;
-        Optional<DeprecatedString> close_message;
+        Optional<String> close_message;
         auto title = "Running Process"sv;
         if (tty_has_foreground_process()) {
-            close_message = "Close Terminal and kill its foreground process?";
+            close_message = "Close Terminal and kill its foreground process?"_string.release_value_but_fixme_should_propagate_errors();
         } else {
             auto child_process_count = shell_child_process_count();
             if (child_process_count > 1) {
                 title = "Running Processes"sv;
-                close_message = DeprecatedString::formatted("Close Terminal and kill its {} background processes?", child_process_count);
+                close_message = String::formatted("Close Terminal and kill its {} background processes?", child_process_count).release_value_but_fixme_should_propagate_errors();
             } else if (child_process_count == 1) {
-                close_message = "Close Terminal and kill its background process?";
+                close_message = "Close Terminal and kill its background process?"_string.release_value_but_fixme_should_propagate_errors();
             }
         }
         if (close_message.has_value())
@@ -471,6 +465,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         modified_state_check_timer->start();
     int result = app->exec();
     dbgln("Exiting terminal, updating utmp");
-    utmp_update(ptsname, 0, false);
+    TRY(utmp_update(ptsname, 0, false));
     return result;
 }