Code cleanup

2 files changed, 168 insertions, 175 deletions

diff --git a/ProcessManager.pro b/ProcessManager.pro
index 95bc758..51dd971 100644
--- a/ProcessManager.pro
+++ b/ProcessManager.pro
@@ -5,4 +5,4 @@ CONFIG -= qt
 
 SOURCES += main.c
 
-QMAKE_CFLAGS *= -std=c11
+QMAKE_CFLAGS *= -std=c11 -Wall -Werror
diff --git a/main.c b/main.c
index 418b1cb..e9a6f80 100644
--- a/main.c
+++ b/main.c
@@ -4,6 +4,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
@@ -14,19 +15,21 @@
 #include <errno.h>
 #include <limits.h>
 
-char stdinName[PATH_MAX];
-char stdoutName[PATH_MAX];
-char stderrName[PATH_MAX];
-char ctrlinName[PATH_MAX];
-char ctrloutName[PATH_MAX];
+#define BUF_BLOCK_SIZE 4096
 
-char progRunning = 0;
-char progQuit = 0;
+char stdinName[2 * PATH_MAX];
+char stdoutName[2 * PATH_MAX];
+char stderrName[2 * PATH_MAX];
+char ctrlinName[2 * PATH_MAX];
+char ctrloutName[2 * PATH_MAX];
+
+bool progRunning = false;
+bool progQuit = false;
 sig_atomic_t progStatus = 0;
 
 pid_t child = 0;
 
-char * normalize_path(const char * src, char * res)
+char * normalizePath(const char * src, char * res)
 {
     size_t src_len = strlen(src);
     size_t res_len;
@@ -47,13 +50,11 @@ char * normalize_path(const char * src, char * res)
         }
 
         pwd_len = strlen(pwd);
-        //res = malloc(pwd_len + 1 + src_len + 1);
         memcpy(res, pwd, pwd_len);
         res_len = pwd_len;
     }
     else
     {
-        //res = malloc((src_len > 0 ? src_len : 1) + 1);
         res_len = 0;
     }
 
@@ -103,35 +104,20 @@ char * normalize_path(const char * src, char * res)
 
 void cleanup(void)
 {
-    if(stdinName != 0)
-    {
-        remove(stdinName);
-    }
-    if(stdoutName != 0)
-    {
-        remove(stdoutName);
-    }
-    if(stderrName != 0)
-    {
-        remove(stderrName);
-    }
-    if(ctrlinName != 0)
-    {
-        remove(ctrlinName);
-    }
-    if(ctrloutName != 0)
-    {
-        remove(ctrloutName);
-    }
+    remove(stdinName);
+    remove(stdoutName);
+    remove(stderrName);
+    remove(ctrlinName);
+    remove(ctrloutName);
 }
 
-void sigchld_handler(int sig)
+void sigchildHandler(int sig)
 {
     (void)sig;
     wait(&progStatus);
     if(WIFEXITED(progStatus) || WIFSIGNALED(progStatus) || WTERMSIG(progStatus))
     {
-        progQuit = 1;
+        progQuit = true;
     }
 }
 
@@ -143,131 +129,135 @@ void exitHandler(int sig)
     exit(0);
 }
 
-int main(int argc, char *argv[])
+void signalHandler(int sig, void(*func)(int))
 {
-    if(argc != 2)
-    {
-        fprintf(stderr, "Usage: %s FIFONAME_PREFIX\n", argv[0]);
-        return 1;
-    }
-
-    const char* fifonamePrefix = argv[1];
-
-    char tempstdinName[PATH_MAX];
-    char tempstdoutName[PATH_MAX];
-    char tempstderrName[PATH_MAX];
-    char tempctrlinName[PATH_MAX];
-    char tempctrloutName[PATH_MAX];
-
-    sprintf(tempstdinName, "%sstdin", fifonamePrefix);
-    realpath(tempstdinName, stdinName);
-    sprintf(tempstdoutName, "%sstdout", fifonamePrefix);
-    realpath(tempstdoutName, stdoutName);
-    sprintf(tempstderrName, "%sstderr", fifonamePrefix);
-    realpath(tempstderrName, stderrName);
-    sprintf(tempctrlinName, "%sctrlin", fifonamePrefix);
-    realpath(tempctrlinName, ctrlinName);
-    sprintf(tempctrloutName, "%sctrlout", fifonamePrefix);
-    realpath(tempctrloutName, ctrloutName);
-
-    cleanup();
-
-    //signal(SIGCHLD, sigchld_hanlder);
     struct sigaction sa;
-    sa.sa_handler = sigchld_handler;
     sigemptyset(&sa.sa_mask);
     sa.sa_flags = 0;
-    if(sigaction(SIGCHLD, &sa, 0) == -1)
+    sa.sa_handler = func;
+
+    if(sigaction(sig, &sa, 0) == -1)
     {
         perror("sigaction");
         cleanup();
-        return 1;
+        exit(1);
     }
+}
 
-    signal(SIGPIPE, SIG_IGN);
-    signal(SIGUSR1, SIG_IGN);
-    signal(SIGUSR2, SIG_IGN);
-    signal(SIGINT, exitHandler);
-    signal(SIGHUP, exitHandler);
-    signal(SIGQUIT, exitHandler);
-    signal(SIGTERM, exitHandler);
-
-    mode_t origMask = umask(0000);
+void makeFifo(const char* prefix, const char* name, char* nameBuf)
+{
+    strncpy(nameBuf, prefix, PATH_MAX);
+    strncat(nameBuf, name, PATH_MAX);
 
-    if(mkfifo(stdinName, 0666) == -1)
+    if(access(nameBuf, F_OK) == 0)
     {
-        perror("stdinfifo");
-        cleanup();
-        return 1;
+        if(remove(nameBuf) != 0)
+        {
+            perror("remove");
+            cleanup();
+            exit(3);
+        }
     }
-    int stdinFifo = -1; // = open(stdinName, O_RDONLY | O_NONBLOCK);
-    if(mkfifo(stdoutName, 0666) == -1)
+
+    if(mkfifo(nameBuf, 0666) == -1)
     {
-        perror("stdoutfifo");
+        perror("mkfifo");
         cleanup();
-        return 1;
+        exit(2);
     }
-    int stdoutFifo = -1;// = open(stdoutName, O_WRONLY);
-    if(mkfifo(stderrName, 0666) == -1)
-    {
-        perror("stdoutfifo");
-        cleanup();
-        return 1;
-    }
-    int stderrFifo = -1;// = open(stderrName, O_WRONLY);
-    if(mkfifo(ctrlinName, 0666) == -1)
+}
+
+void reopenCtrlinFifo(const char* name, int* fd)
+{
+    // fifo "connection" was lost (e.g. the controlling application quit). reopen it
+    close(*fd);
+    if((*fd = open(name, O_RDONLY)) < 0)
     {
-        perror("ctrlinfifo");
+        perror("reopen ctrlinFifo");
         cleanup();
-        return 1;
+        exit(6);
     }
-    if(mkfifo(ctrloutName, 0666) == -1)
+}
+
+int main(int argc, char *argv[])
+{
+    if(argc != 2)
     {
-        perror("ctrloutfifo");
-        cleanup();
+        fprintf(stderr, "Usage: %s FIFONAME_PREFIX\n", argv[0]);
         return 1;
     }
 
+    signalHandler(SIGCHLD, sigchildHandler);
+
+    signalHandler(SIGPIPE, SIG_IGN);
+    signalHandler(SIGUSR1, SIG_IGN);
+    signalHandler(SIGUSR2, SIG_IGN);
+
+    signalHandler(SIGINT, exitHandler);
+    signalHandler(SIGHUP, exitHandler);
+    signalHandler(SIGQUIT, exitHandler);
+    signalHandler(SIGTERM, exitHandler);
+
+    mode_t origMask = umask(0000);
+
+    char fifonamePrefix[PATH_MAX];
+    realpath(argv[1], fifonamePrefix);
+
+    makeFifo(fifonamePrefix, "stdin", stdinName);
+    makeFifo(fifonamePrefix, "stdout", stdoutName);
+    makeFifo(fifonamePrefix, "stderr", stderrName);
+    makeFifo(fifonamePrefix, "ctrlin", ctrlinName);
+    makeFifo(fifonamePrefix, "ctrlout", ctrloutName);
+
     umask(origMask);
 
+    int stdinFifo = -1, stdoutFifo = -1, stderrFifo = -1;
+
     int ctrlinFifo = open(ctrlinName, O_RDONLY);
-    int ctrloutFifo = open(ctrloutName, O_WRONLY);
-    if(ctrloutFifo == -1 || ctrlinFifo == -1)
+    if(ctrlinFifo == -1)
     {
         perror("open");
         cleanup();
-        return 1;
+        return 3;
+    }
+
+    FILE* ctrloutFile = fopen(ctrloutName, "w");
+    if(ctrloutFile == NULL)
+    {
+        perror("open");
+        cleanup();
+        return 4;
     }
-    FILE* ctrloutFile = fdopen(ctrloutFifo, "w");
     setlinebuf(ctrloutFile);
+
+    // avoid blocking and/or not open errors
     open(ctrloutName, O_RDONLY);
 
-    char* buf = (char*)malloc(4096 * sizeof(char));
-    if(buf == 0)
+    size_t bufSize = BUF_BLOCK_SIZE;
+    char* buf = malloc(bufSize* sizeof(char));
+    if(buf == NULL)
     {
         perror("malloc");
         cleanup();
-        return 1;
+        return 5;
     }
-    int bufSize = 4096, curPos = 0;
-    char shouldExit = 0;
-    for(;!shouldExit;)
+
+    for(;;)
     {
         char* newlinePos = 0;
         if(progQuit && progRunning)
         {
-            progRunning = 0;
-            progQuit = 0;
+            progRunning = true;
+            progQuit = false;
             fprintf(ctrloutFile, "QUIT: %d\n", WEXITSTATUS(progStatus));
             close(stdinFifo);
             close(stdoutFifo);
             close(stderrFifo);
         }
-        int count = read(ctrlinFifo, (void*)buf, 4096);
-        if(count == 0 || count == -1)
+        ssize_t count = read(ctrlinFifo, buf, BUF_BLOCK_SIZE - 1);
+        if(count <= 0)
         {
-            close(ctrlinFifo);
-            ctrlinFifo = open(ctrlinName, O_RDONLY);
+            reopenCtrlinFifo(ctrlinName, &ctrlinFifo);
         }
         else
         {
@@ -291,42 +281,45 @@ int main(int argc, char *argv[])
                 }
                 else if(strncmp("START ", buf, 6) == 0)
                 {
-                    if(progRunning && ctrloutFile != 0)
+                    if(progRunning && ctrloutFile != NULL)
                     {
                         fputs("ALREADY\n", ctrloutFile);
                     }
                     else
                     {
+                        int curPos = 0;
                         char* prog = buf + 6;
-                        while((newlinePos = strchr(prog, '\n')) == 0)
+                        while((newlinePos = strchr(prog, '\n')) == NULL)
                         {
                             bufSize += count;
 
-                            buf = (char*)realloc((void*)buf, bufSize);
-                            if(buf == 0)
+                            buf = realloc(buf, bufSize);
+                            if(buf == NULL)
                             {
                                 perror("realloc");
                                 cleanup();
-                                return 1;
+                                return 7;
                             }
-                            count = read(ctrlinFifo, (void*)(buf+curPos), 4096);
+
+                            count = read(ctrlinFifo, buf + curPos, BUF_BLOCK_SIZE - 1);
                             if(count == 0)
                             {
-                                close(ctrlinFifo);
-                                ctrlinFifo = open(ctrlinName, O_RDONLY);
+                                reopenCtrlinFifo(ctrlinName, &ctrlinFifo);
                                 break;
                             }
                             curPos += count;
                         }
                         *newlinePos = '\0';
-                        char** nargv = (char**)malloc(2 * sizeof(char*));
-                        if(nargv == 0)
+
+                        char** nargv = malloc(2 * sizeof(char*));
+                        if(nargv == NULL)
                         {
                             perror("malloc");
                             cleanup();
-                            return 1;
+                            return 8;
                         }
                         nargv[0] = prog;
+
                         int nargc = 1;
                         unsigned int len = strlen(prog);
                         for(unsigned int i = 0; i < len; ++i)
@@ -339,77 +332,82 @@ int main(int argc, char *argv[])
                             else if(prog[i] == ' ')
                             {
                                 prog[i] = '\0';
-                                nargv = (char**)realloc((void*)nargv, (++nargc + 1) * sizeof(char*));
-                                if(nargv == 0)
+                                nargv = realloc(nargv, (++nargc + 1) * sizeof(char*));
+                                if(nargv == NULL)
                                 {
                                     perror("realloc");
                                     cleanup();
-                                    return 1;
+                                    return 9;
                                 }
                                 nargv[nargc - 1] = prog + i + 1;
                             }
                         }
-                        nargv[nargc] = 0;
+                        nargv[nargc] = NULL;
 
                         char absPath[PATH_MAX];
-                        nargv[0] = normalize_path(nargv[0], absPath);
+                        nargv[0] = normalizePath(nargv[0], absPath);
 
-                        if(nargv[0] == 0)
+                        if(nargv[0] == NULL || access(nargv[0], X_OK) != 0)
                         {
                             fprintf(ctrloutFile, "STARTFAIL: %s\n", strerror(errno));
                         }
+                        else
+                        {
+                            progRunning = true;
 
-                        progRunning = 1;
+                            child = fork();
 
-                        child = fork();
-                        if(child == -1)
-                        {
-                            perror("fork");
-                            cleanup();
-                            return 1;
-                        }
-                        if(child == 0)
-                        {
-                            stdinFifo = open(stdinName, O_RDONLY);
-                            if(stdinFifo == -1)
-                            {
-                                perror("open");
-                            }
-                            stdoutFifo = open(stdoutName, O_WRONLY);
-                            if(stdoutFifo == -1)
+                            if(child == -1)
                             {
-                                perror("open");
+                                perror("fork");
+                                cleanup();
+                                return 10;
                             }
-                            stderrFifo = open(stderrName, O_WRONLY);
-                            if(stderrFifo == -1)
+
+                            if(child == 0)
                             {
-                                perror("open");
+                                stdinFifo = open(stdinName, O_RDONLY);
+                                if(stdinFifo == -1)
+                                {
+                                    perror("open");
+                                }
+                                stdoutFifo = open(stdoutName, O_WRONLY);
+                                if(stdoutFifo == -1)
+                                {
+                                    perror("open");
+                                }
+                                stderrFifo = open(stderrName, O_WRONLY);
+                                if(stderrFifo == -1)
+                                {
+                                    perror("open");
+                                }
+                                dup2(stdinFifo, STDIN_FILENO);
+                                dup2(stdoutFifo, STDOUT_FILENO);
+                                dup2(stderrFifo, STDERR_FILENO);
+                                if(execvp(nargv[0], nargv) == -1)
+                                {
+                                    perror("execvp");
+                                    exit(11);
+                                }
                             }
-                            dup2(stdinFifo, STDIN_FILENO);
-                            dup2(stdoutFifo, STDOUT_FILENO);
-                            dup2(stderrFifo, STDERR_FILENO);
-                            if(execvp(nargv[0], nargv) == -1)
+                            else
                             {
-                                perror("execvp");
-                                exit(1);
+                                stdinFifo = open(stdinName, O_WRONLY);
+                                stdoutFifo = open(stdoutName, O_RDONLY);
+                                stderrFifo = open(stderrName, O_RDONLY);
+                                fprintf(ctrloutFile, "RUNNING: %d\n", child);
                             }
                         }
-                        else
-                        {
-                            stdinFifo = open(stdinName, O_WRONLY);
-                            stdoutFifo = open(stdoutName, O_RDONLY);
-                            stderrFifo = open(stderrName, O_RDONLY);
-                            fprintf(ctrloutFile, "RUNNING: %d\n", child);
-                        }
 
-                        bufSize = 4096;
-                        buf = (char*)realloc((void*)buf, bufSize);
-                        if(buf == 0)
+                        bufSize = BUF_BLOCK_SIZE;
+                        buf = realloc(buf, bufSize);
+                        if(buf == NULL)
                         {
                             perror("realloc");
                             cleanup();
-                            return 1;
+                            return 12;
                         }
+                        buf[0] = '\0';
                     }
                 }
                 else if(strncmp(buf, "CD ", 3) == 0)
@@ -432,8 +430,8 @@ int main(int argc, char *argv[])
                 }
                 else if(strcmp(buf, "EXIT\n") == 0)
                 {
-                    shouldExit = 1;
-                    break;
+                    cleanup();
+                    return 0;
                 }
                 else if(strcmp(buf, "CLOSE\n") == 0)
                 {
@@ -456,7 +454,7 @@ int main(int argc, char *argv[])
                 }
                 if(newlinePos != 0)
                 {
-                    strcpy(buf, newlinePos + 1);
+                    memmove(buf, newlinePos + 1, strlen(newlinePos + 1));
                 }
                 else
                 {
@@ -466,9 +464,4 @@ int main(int argc, char *argv[])
             while(buf[0] != '\0');
         }
     }
-
-    cleanup();
-
-    return 0;
 }
-