Shell: Support semicolons for separating commands

Shell/Parser.cpp

@@ -22,6 +22,13 @@ void Parser::commit_subcommand()
     m_subcommands.append({ move(m_tokens), move(m_redirections), {} });
 }
 
+void Parser::commit_command()
+{
+    if (m_subcommands.is_empty())
+        return;
+    m_commands.append({ move(m_subcommands) });
+}
+
 void Parser::do_pipe()
 {
     m_redirections.append({ Redirection::Pipe, STDOUT_FILENO });
@@ -38,7 +45,7 @@ void Parser::begin_redirect_write(int fd)
     m_redirections.append({ Redirection::FileWrite, fd });
 }
 
-Vector<Subcommand> Parser::parse()
+Vector<Command> Parser::parse()
 {
     for (int i = 0; i < m_input.length(); ++i) {
         char ch = m_input.characters()[i];
@@ -48,6 +55,12 @@ Vector<Subcommand> Parser::parse()
                 commit_token();
                 break;
             }
+            if (ch == ';') {
+                commit_token();
+                commit_subcommand();
+                commit_command();
+                break;
+            }
             if (ch == '|') {
                 commit_token();
                 if (m_tokens.is_empty()) {
@@ -140,6 +153,7 @@ Vector<Subcommand> Parser::parse()
     }
     commit_token();
     commit_subcommand();
+    commit_command();
 
     if (!m_subcommands.is_empty()) {
         for (auto& redirection : m_subcommands.last().redirections) {
@@ -150,5 +164,5 @@ Vector<Subcommand> Parser::parse()
         }
     }
 
-    return move(m_subcommands);
+    return move(m_commands);
 }

Shell/Parser.h

@@ -27,6 +27,10 @@ struct Subcommand {
     Vector<Rewiring> rewirings;
 };
 
+struct Command {
+    Vector<Subcommand> subcommands;
+};
+
 class Parser {
 public:
     explicit Parser(const String& input)
@@ -34,11 +38,12 @@ public:
     {
     }
 
-    Vector<Subcommand> parse();
+    Vector<Command> parse();
 
 private:
     void commit_token();
     void commit_subcommand();
+    void commit_command();
     void do_pipe();
     void begin_redirect_read(int fd);
     void begin_redirect_write(int fd);
@@ -53,6 +58,7 @@ private:
     State m_state { Free };
     String m_input;
 
+    Vector<Command> m_commands;
     Vector<Subcommand> m_subcommands;
     Vector<String> m_tokens;
     Vector<Redirection> m_redirections;

Shell/main.cpp

@@ -20,7 +20,7 @@
 #include <termios.h>
 #include <unistd.h>
 
-//#define SH_DEBUG
+#define SH_DEBUG
 
 GlobalState g;
 static LineEditor editor;
@@ -320,97 +320,43 @@ static int run_command(const String& cmd)
     if (cmd.is_empty())
         return 0;
 
-    auto subcommands = Parser(cmd).parse();
+    auto commands = Parser(cmd).parse();
 
 #ifdef SH_DEBUG
-    for (int i = 0; i < subcommands.size(); ++i) {
+    for (auto& command : commands) {
-        for (int j = 0; j < i; ++j)
+        for (int i = 0; i < command.subcommands.size(); ++i) {
-            dbgprintf("    ");
-        for (auto& arg : subcommands[i].args) {
-            dbgprintf("<%s> ", arg.characters());
-        }
-        dbgprintf("\n");
-        for (auto& redirecton : subcommands[i].redirections) {
             for (int j = 0; j < i; ++j)
                 dbgprintf("    ");
-            dbgprintf("  ");
+            for (auto& arg : command.subcommands[i].args) {
-            switch (redirecton.type) {
+                dbgprintf("<%s> ", arg.characters());
-            case Redirection::Pipe:
+            }
-                dbgprintf("Pipe\n");
+            dbgprintf("\n");
-                break;
+            for (auto& redirecton : command.subcommands[i].redirections) {
-            case Redirection::FileRead:
+                for (int j = 0; j < i; ++j)
-                dbgprintf("fd:%d = FileRead: %s\n", redirecton.fd, redirecton.path.characters());
+                    dbgprintf("    ");
-                break;
+                dbgprintf("  ");
-            case Redirection::FileWrite:
+                switch (redirecton.type) {
-                dbgprintf("fd:%d = FileWrite: %s\n", redirecton.fd, redirecton.path.characters());
+                case Redirection::Pipe:
-                break;
+                    dbgprintf("Pipe\n");
-            case Redirection::FileWriteAppend:
+                    break;
-                dbgprintf("fd:%d = FileWriteAppend: %s\n", redirecton.fd, redirecton.path.characters());
+                case Redirection::FileRead:
-                break;
+                    dbgprintf("fd:%d = FileRead: %s\n", redirecton.fd, redirecton.path.characters());
-            default:
+                    break;
-                break;
+                case Redirection::FileWrite:
+                    dbgprintf("fd:%d = FileWrite: %s\n", redirecton.fd, redirecton.path.characters());
+                    break;
+                case Redirection::FileWriteAppend:
+                    dbgprintf("fd:%d = FileWriteAppend: %s\n", redirecton.fd, redirecton.path.characters());
+                    break;
+                default:
+                    break;
+                }
             }
         }
+        dbgprintf("\n");
     }
 #endif
 
-    if (subcommands.is_empty())
-        return 0;
-
-    FileDescriptionCollector fds;
-
-    for (int i = 0; i < subcommands.size(); ++i) {
-        auto& subcommand = subcommands[i];
-        for (auto& redirection : subcommand.redirections) {
-            switch (redirection.type) {
-            case Redirection::Pipe: {
-                int pipefd[2];
-                int rc = pipe(pipefd);
-                if (rc < 0) {
-                    perror("pipe");
-                    return 1;
-                }
-                subcommand.rewirings.append({ STDOUT_FILENO, pipefd[1] });
-                auto& next_command = subcommands[i + 1];
-                next_command.rewirings.append({ STDIN_FILENO, pipefd[0] });
-                fds.add(pipefd[0]);
-                fds.add(pipefd[1]);
-                break;
-            }
-            case Redirection::FileWriteAppend: {
-                int fd = open(redirection.path.characters(), O_WRONLY | O_CREAT | O_APPEND, 0666);
-                if (fd < 0) {
-                    perror("open");
-                    return 1;
-                }
-                subcommand.rewirings.append({ redirection.fd, fd });
-                fds.add(fd);
-                break;
-            }
-            case Redirection::FileWrite: {
-                int fd = open(redirection.path.characters(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
-                if (fd < 0) {
-                    perror("open");
-                    return 1;
-                }
-                subcommand.rewirings.append({ redirection.fd, fd });
-                fds.add(fd);
-                break;
-            }
-            case Redirection::FileRead: {
-                int fd = open(redirection.path.characters(), O_RDONLY);
-                if (fd < 0) {
-                    perror("open");
-                    return 1;
-                }
-                subcommand.rewirings.append({ redirection.fd, fd });
-                fds.add(fd);
-                break;
-            }
-            }
-        }
-    }
-
     struct termios trm;
     tcgetattr(0, &trm);
 
@@ -419,99 +365,159 @@ static int run_command(const String& cmd)
         pid_t pid;
     };
 
-    Vector<SpawnedProcess> children;
-
-    CommandTimer timer;
-
-    for (int i = 0; i < subcommands.size(); ++i) {
-        auto& subcommand = subcommands[i];
-        Vector<String> argv_string = process_arguments(subcommand.args);
-        Vector<const char*> argv;
-        argv.ensure_capacity(argv_string.size());
-        for (const auto& s : argv_string) {
-            argv.append(s.characters());
-        }
-        argv.append(nullptr);
-
-#ifdef SH_DEBUG
-        for (auto& arg : argv) {
-            dbgprintf("<%s> ", arg);
-        }
-        dbgprintf("\n");
-#endif
-
-        int retval = 0;
-        if (handle_builtin(argv.size() - 1, const_cast<char**>(argv.data()), retval))
-            return retval;
-
-        pid_t child = fork();
-        if (!child) {
-            setpgid(0, 0);
-            tcsetpgrp(0, getpid());
-            for (auto& rewiring : subcommand.rewirings) {
-#ifdef SH_DEBUG
-                dbgprintf("in %s<%d>, dup2(%d, %d)\n", argv[0], getpid(), redirection.rewire_fd, redirection.fd);
-#endif
-                int rc = dup2(rewiring.rewire_fd, rewiring.fd);
-                if (rc < 0) {
-                    perror("dup2");
-                    return 1;
-                }
-            }
-
-            fds.collect();
-
-            int rc = execvp(argv[0], const_cast<char* const*>(argv.data()));
-            if (rc < 0) {
-                if (errno == ENOENT)
-                    fprintf(stderr, "%s: Command not found.\n", argv[0]);
-                else
-                    fprintf(stderr, "execvp(%s): %s\n", argv[0], strerror(errno));
-                exit(1);
-            }
-            ASSERT_NOT_REACHED();
-        }
-        children.append({ argv[0], child });
-    }
-
-#ifdef SH_DEBUG
-    dbgprintf("Closing fds in shell process:\n");
-#endif
-    fds.collect();
-
-#ifdef SH_DEBUG
-    dbgprintf("Now we gotta wait on children:\n");
-    for (auto& child : children)
-        dbgprintf("  %d\n", child);
-#endif
-
-    int wstatus = 0;
     int return_value = 0;
 
-    for (int i = 0; i < children.size(); ++i) {
+    for (auto& command : commands) {
-        auto& child = children[i];
+        if (command.subcommands.is_empty())
-        do {
+            continue;
-            int rc = waitpid(child.pid, &wstatus, WEXITED | WSTOPPED);
+
-            if (rc < 0 && errno != EINTR) {
+        FileDescriptionCollector fds;
-                if (errno != ECHILD)
+
-                    perror("waitpid");
+        for (int i = 0; i < command.subcommands.size(); ++i) {
-                break;
+            auto& subcommand = command.subcommands[i];
-            }
+            for (auto& redirection : subcommand.redirections) {
-            if (WIFEXITED(wstatus)) {
+                switch (redirection.type) {
-                if (WEXITSTATUS(wstatus) != 0)
+                case Redirection::Pipe: {
-                    dbg() << "Shell: " << child.name << ":" << child.pid << " exited with status " << WEXITSTATUS(wstatus);
+                    int pipefd[2];
-                if (i == 0)
+                    int rc = pipe(pipefd);
-                    return_value = WEXITSTATUS(wstatus);
+                    if (rc < 0) {
-            } else if (WIFSTOPPED(wstatus)) {
+                        perror("pipe");
-                printf("Shell: %s(%d) stopped.\n", child.name.characters(), child.pid);
+                        return 1;
-            } else {
+                    }
-                if (WIFSIGNALED(wstatus)) {
+                    subcommand.rewirings.append({ STDOUT_FILENO, pipefd[1] });
-                    printf("Shell: %s(%d) exited due to signal '%s'\n", child.name.characters(), child.pid, strsignal(WTERMSIG(wstatus)));
+                    auto& next_command = command.subcommands[i + 1];
-                } else {
+                    next_command.rewirings.append({ STDIN_FILENO, pipefd[0] });
-                    printf("Shell: %s(%d) exited abnormally\n", child.name.characters(), child.pid);
+                    fds.add(pipefd[0]);
+                    fds.add(pipefd[1]);
+                    break;
+                }
+                case Redirection::FileWriteAppend: {
+                    int fd = open(redirection.path.characters(), O_WRONLY | O_CREAT | O_APPEND, 0666);
+                    if (fd < 0) {
+                        perror("open");
+                        return 1;
+                    }
+                    subcommand.rewirings.append({ redirection.fd, fd });
+                    fds.add(fd);
+                    break;
+                }
+                case Redirection::FileWrite: {
+                    int fd = open(redirection.path.characters(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+                    if (fd < 0) {
+                        perror("open");
+                        return 1;
+                    }
+                    subcommand.rewirings.append({ redirection.fd, fd });
+                    fds.add(fd);
+                    break;
+                }
+                case Redirection::FileRead: {
+                    int fd = open(redirection.path.characters(), O_RDONLY);
+                    if (fd < 0) {
+                        perror("open");
+                        return 1;
+                    }
+                    subcommand.rewirings.append({ redirection.fd, fd });
+                    fds.add(fd);
+                    break;
+                }
                 }
             }
-        } while (errno == EINTR);
+        }
+
+        Vector<SpawnedProcess> children;
+
+        CommandTimer timer;
+
+        for (int i = 0; i < command.subcommands.size(); ++i) {
+            auto& subcommand = command.subcommands[i];
+            Vector<String> argv_string = process_arguments(subcommand.args);
+            Vector<const char*> argv;
+            argv.ensure_capacity(argv_string.size());
+            for (const auto& s : argv_string) {
+                argv.append(s.characters());
+            }
+            argv.append(nullptr);
+
+#ifdef SH_DEBUG
+            for (auto& arg : argv) {
+                dbgprintf("<%s> ", arg);
+            }
+            dbgprintf("\n");
+#endif
+
+            int retval = 0;
+            if (handle_builtin(argv.size() - 1, const_cast<char**>(argv.data()), retval))
+                return retval;
+
+            pid_t child = fork();
+            if (!child) {
+                setpgid(0, 0);
+                tcsetpgrp(0, getpid());
+                for (auto& rewiring : subcommand.rewirings) {
+#ifdef SH_DEBUG
+                    dbgprintf("in %s<%d>, dup2(%d, %d)\n", argv[0], getpid(), rewiring.rewire_fd, rewiring.fd);
+#endif
+                    int rc = dup2(rewiring.rewire_fd, rewiring.fd);
+                    if (rc < 0) {
+                        perror("dup2");
+                        return 1;
+                    }
+                }
+
+                fds.collect();
+
+                int rc = execvp(argv[0], const_cast<char* const*>(argv.data()));
+                if (rc < 0) {
+                    if (errno == ENOENT)
+                        fprintf(stderr, "%s: Command not found.\n", argv[0]);
+                    else
+                        fprintf(stderr, "execvp(%s): %s\n", argv[0], strerror(errno));
+                    exit(1);
+                }
+                ASSERT_NOT_REACHED();
+            }
+            children.append({ argv[0], child });
+        }
+
+#ifdef SH_DEBUG
+        dbgprintf("Closing fds in shell process:\n");
+#endif
+        fds.collect();
+
+#ifdef SH_DEBUG
+        dbgprintf("Now we gotta wait on children:\n");
+        for (auto& child : children)
+            dbgprintf("  %d\n", child);
+#endif
+
+        int wstatus = 0;
+
+        for (int i = 0; i < children.size(); ++i) {
+            auto& child = children[i];
+            do {
+                int rc = waitpid(child.pid, &wstatus, WEXITED | WSTOPPED);
+                if (rc < 0 && errno != EINTR) {
+                    if (errno != ECHILD)
+                        perror("waitpid");
+                    break;
+                }
+                if (WIFEXITED(wstatus)) {
+                    if (WEXITSTATUS(wstatus) != 0)
+                        dbg() << "Shell: " << child.name << ":" << child.pid << " exited with status " << WEXITSTATUS(wstatus);
+                    if (i == 0)
+                        return_value = WEXITSTATUS(wstatus);
+                } else if (WIFSTOPPED(wstatus)) {
+                    printf("Shell: %s(%d) stopped.\n", child.name.characters(), child.pid);
+                } else {
+                    if (WIFSIGNALED(wstatus)) {
+                        printf("Shell: %s(%d) exited due to signal '%s'\n", child.name.characters(), child.pid, strsignal(WTERMSIG(wstatus)));
+                    } else {
+                        printf("Shell: %s(%d) exited abnormally\n", child.name.characters(), child.pid);
+                    }
+                }
+            } while (errno == EINTR);
+        }
     }
 
     // FIXME: Should I really have to tcsetpgrp() after my child has exited?