2
\$\begingroup\$

Like many others I've been writing a shell command-line interpreter that can do a pretty decent pipeline parsing, first it splits the command at pipeline char (|), then it splits the substring at unquoted whitespace so that a pipeline e.g. $ ls -a -h -l|awk'{print $3}'|sort -n gets represented with a matrix A where A[i][j] is program (pipe) i and argument number j. The code produces the correct output except for quoted pipelines (e.g. $ echo 'foo|bar'|cat won't work) but that functionality I can add. What I want is to make the code more readable and maintainable now that it "works" (doesn't crash and makes the right output).

main.c

#define _XOPEN_SOURCE 500

#include <sys/stat.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/wait.h>
#include "openshell.h"
#include "errors.h"
#include "do.h"
#include "CommandEntry.h"
#include <errno.h>
#include <readline/readline.h>
#include <unistd.h>
#include <readline/history.h>
#include <histedit.h>

#ifdef SIGDET
#if SIGDET == 1
int isSignal = 1;       /*Termination detected by signals*/
#endif
#endif
static int count = 0;
static FILE *sourcefiles[MAX_SOURCE];

/*
 * The special maximum argument value which means that there is
 * no limit to the number of arguments on the command line.
 */
#define    INFINITE_ARGS    0x7fffffff

static struct option options[] = {
        {"with_param", 1, 0, 'p'},
        {"version",    0, 0, 'v'},
        {"help",       0, 0, 'h'},
        {0,            0, 0, 0}
};
/*
 * The table of built-in commands.
 * A command is terminated wih an entry containing NULL values.
 * These commands should preferable by written in openshell
 */
static const CommandEntry commandEntryTable[] =
        {
                {
                        "checkenv", do_checkenv, 1, INFINITE_ARGS,
                        "Check environment variables",
                        ""
                },

                {
                        "editenv",  do_editenv,  3, INFINITE_ARGS,
                        "do_editenv",
                        "[txp]v arFileName fileName ..."
                },

                {
                        "cd",       do_cd,       1, 2,
                        "Change current directory",
                        "[dirName]"
                },


                {
                        "exit",     do_exit,     1, 2,
                        "Exit from shell",
                        "[exit value]"
                },


                {
                        "help",     do_help,     1, 2,
                        "Print help about a command",
                        "[word]"
                },

                {
                        "killport", do_killport, 2, INFINITE_ARGS,
                        "Send a signal to the specified process",
                        "[-sig] pid ..."
                },


                {
                        NULL,       0,           0, 0,
                        NULL,
                        NULL
                }
        };


char *concat(char *s1, char *s2) {
    char *result = malloc(strlen(s1) + strlen(s2) + 1);//+1 for the zero-terminator
    //in real code you would check for errors in malloc here
    if (result == NULL) {
        fprintf(stderr, "malloc failed!\n");
        return (char *) '0';
    }
    strcpy(result, s1);
    strcat(result, s2);
    return result;
}

int testFn(const char *str) {
    return (str && *str && str[strlen(str) - 1] == '}') ? 1 : 0;
}

static int runCmd(const char *cmd) {
    const char *cp;
    pid_t pid;
    int status;
    struct command structcommand[15];
    char **argv = 0;
    int argc = 1;
    bool pipe = false;
    char *string[75][75];
    char *pString3[40];
    char *pString2[40];
    int n = 0;
    char **ptr1;
    char string1[75];
    bool keep = false;
    char *pString1[75];
    char *pString[75];
    *pString1 = "\0";
    *pString = "\0";
    char temp[75] = {'\0'};
    int w = 0;
    bool b = false;
    int j = 0;
    int i;
    int p = 0;
    char **ptr;
    char *tmpchar;
    char *cmdtmp;
    bool b1 = false;
    char *dest;
    int y = 0;
    i = 0;
    int h = 0;
    for (int x = 0; x < 75; x++) { /* for each pipeline */
        for (int c = 0; c < 75; c++) { /* for each pipeline */
            string[x][c] = '\0';
        }
    }
    if (cmd) {
        for (cp = cmd; *cp; cp++) {
            if ((*cp >= 'a') && (*cp <= 'z')) {
                continue;
            }
            if ((*cp >= 'A') && (*cp <= 'Z')) {
                continue;
            }
            if (isDecimal(*cp)) {
                continue;
            }
            if (isBlank(*cp)) {
                continue;
            }
            if ((*cp == '.') || (*cp == '/') || (*cp == '-') ||
                (*cp == '+') || (*cp == '=') || (*cp == '_') ||
                (*cp == ':') || (*cp == ',') || (*cp == '\'') ||
                (*cp == '"')) {
                continue;
            }
        }
    }
    if (cmd) {
        cmdtmp = malloc(sizeof(char *) * strlen(cmd) + 1);
        strcpy(cmdtmp, cmd);
        tmpchar = malloc(sizeof(char *) * strlen(cmd) + 1);
        if (tmpchar == NULL) {
            printf("Error allocating memory!\n"); /* print an error message */
            return 1; /* return with failure */
        }
        strcpy(tmpchar, cmd);
        ptr1 = str_split(pString3, cmdtmp, '|');
        if (strstr(cmd, "|") == NULL) {         /* not a pipeline */
            makeArgs(cmd, &argc, (const char ***) &argv, pipe, 0, 0);
            for (j = 0; j < argc; j++) {
                string[0][j] = argv[j];
                structcommand[i].argv = string[0]; /*process;*/
            }
            n++;
        }
        else {
            for (i = 0; *(ptr1 + i); i++) { /* tokenize the input string for each pipeline*/
                n++; /* save number of pipelines */
                int e = 0; /* a counter */
                *pString = "\0"; /* should malloc and free this? */
                strcpy(string1, *(ptr1 + i));
                if ((string1[0] != '\0') && !isspace(string1[0])) { /* this is neither the end nor a new argument */
                    ptr = str_split(pString2, *(&string1), ' '); /* split the string at the arguments */
                    h = 0;
                    for (j = 0; *(ptr + j); j++) { /* step through the arguments */
                        /* the pipeline is in cmdtmp and the argument/program is in ptr[i] */
                        if (ptr + j && !b && strstr(*(ptr + j), "'")) {
                            b = true;
                            strcpy(temp, *(ptr + j));
                            if (y < 1) {
                                y++;
                            }
                        }
                        while (b) {
                            if (*(ptr + j) && strstr(*(ptr + j), "'")) { /* end of quote */
                                b = false;
                                if (y < 1) {
                                    string[i][j] = strcpy(temp, *(ptr + j));
                                }
                                y = 0;
                            }
                            else if (*(ptr + j)) { /* read until end of quote */
                                string[i][j] = temp;
                                continue;
                            } else {
                                b = false;
                                break;
                            }
                        }
                        if (ptr + j) {
                            if (*(ptr + j)[0] == '{') {
                                keep = true;
                            }
                            if (testFn(*(ptr + j))) { /* test for last char */
                                string[i][j - p] = concat(*pString1, *(ptr + j));
                                keep = false;
                                free(*pString1);
                                goto mylabel;
                            }
                            if (keep) {
                                *pString1 = concat(*pString1, *(ptr + j));
                                *pString1 = concat(*pString1, " ");
                                p++;
                            } else {
                                strcpy(temp, *(ptr + j));
                                b1 = false;
                                int q = j;
                                for (e = 0; *(ptr + q + e); e++) { /* step through the string */
                                    b1 = true;
                                    if (*(ptr + e + q)) {
                                        *pString = concat(*pString, *(ptr + e + q));
                                        *pString = concat(*pString, " ");
                                    }
                                    j = e;
                                }
                                if (makeArgs(*pString, &argc, (const char ***) &argv, pipe, i, h)) {
                                    for (int r = 0; argv[r] != NULL; r++) {
                                        dest = malloc(sizeof(char *) * strlen(argv[r]) + 1);
                                        *dest = '0';
                                        strcpy(dest, argv[r]);
                                        string[w][r] = dest;
                                    }
                                    w++;

                                } else {
                                    if (!b1) { /* no args (?) */
                                        for (int r = 0; argv[r] != NULL; r++) {
                                            string[i][r] = argv[r];
                                        }

                                    }
                                }

                            }
                        }
                    }
                    mylabel:
                    free(ptr);
                    dump_argv((const char *) "d", argc, argv);
                }
            }
            free(ptr1);
            free(cmdtmp);
            free(tmpchar);
        }
        for (i = 0; i < n; i++) {
            for (j = 0; DEBUG && string[i][j] != NULL; j++) {
                if (i == 0 && j == 0) printf("\n");
                printf("p[%d][%d] %s\n", i, j, string[i][j]);
            }
            structcommand[i].argv = string[i];
        }
        fflush(NULL);
        pid = fork();
        if (pid < 0) {
            perror("fork failed");
            return -1;
        }
        /* If we are the child process, then go execute the string.*/
        if (pid == 0) {
            /* spawn(cmd);*/
            fork_pipes(n, structcommand);
        }
        /*
         * We are the parent process.
         * Wait for the child to complete.
         */
        status = 0;
        while (((pid = waitpid(pid, &status, 0)) < 0) && (errno == EINTR));
        if (pid < 0) {
            fprintf(stderr, "Error from waitpid: %s", strerror(errno));
            return -1;
        }
        if (WIFSIGNALED(status)) {
            fprintf(stderr, "pid %ld: killed by signal %d\n",
                    (long) pid, WTERMSIG(status));

            return -1;
        }
    }
    return WEXITSTATUS(status);


}

/* The shell performs wildcard expansion on each token it extracts while parsing the command line.
 * Oftentimes, globbing will obviously not do anything (for example, ls just returns ls).
 * When you want nullglob behavior you'll have to know whether the glob function actually found any glob characters, though
 */
static void expandVariable(char *shellcommand) {
    char mystring[CMD_LEN];
    char *cp;
    char *ep;

    strcpy(mystring, shellcommand);
    cp = strstr(mystring, "$(");
    if (cp) {
        *cp++ = '\0';
        strcpy(shellcommand, mystring);
        ep = ++cp;
        while (*ep && (*ep != ')')) ep++;
        if (*ep == ')') *ep++ = '\0';
        cp = getenv(cp);
        if (cp) strcat(shellcommand, cp);
        strcat(shellcommand, ep);
    }
    return;
}

int do_help(int argc, const char **argv) {
    const CommandEntry *entry;
    const char *str;

    str = NULL;

    if (argc == 2)
        str = argv[1];

    /*
     * Check for an exact match, in which case describe the program.
     */
    if (str) {
        for (entry = commandEntryTable; entry->name; entry++) {
            if (strcmp(str, entry->name) == 0) {
                printf("%s\n", entry->description);

                printf("usage: %s %s\n", entry->name,
                       entry->usage);

                return 0;
            }
        }
    }

    /*
     * Print short information about commands which contain the
     * specified word.
     */
    for (entry = commandEntryTable; entry->name; entry++) {
        if ((str == NULL) || (strstr(entry->name, str) != NULL) ||
            (strstr(entry->usage, str) != NULL)) {
            printf("%-10s %s\n", entry->name, entry->usage);
        }
    }

    return 0;
}

char s[] = "Interrupt\n";
char *input;

void handler(int signum) {
    input = '\0';
    if (write(fileno(stdin), s, sizeof s - 1)) { } else { }
    if (signum) { if (false); } else { }
}

/*
 * Try to execute a built-in command.
 * Returns TRUE if the command is a built in, whether or not the
 * command succeeds.  Returns FALSE if this is not a built-in command.
 */
bool exec_builtin(const char *cmd) {
    const char *endCmd;
    const CommandEntry *entry;
    int argc;
    const char **argv;
    char cmdName[CMD_LEN];

    /*
     * Look for the end of the command name and then copy the
     * command name to a buffer so we can null terminate it.
     */
    endCmd = cmd;

    if (endCmd)
        while (*endCmd && !isBlank(*endCmd))
            endCmd++;

    memcpy(cmdName, cmd, endCmd - cmd);
    cmdName[endCmd - cmd] = '\0';

    /*
     * Search the command table looking for the command name.
     */
    for (entry = commandEntryTable; entry->name != NULL; entry++) {
        if (strcmp(entry->name, cmdName) == 0)
            break;
    }

    /*
     * If the command is not a built-in, return indicating that.
     */
    if (entry->name == NULL) {
        return false;
    }

    bool bo = false;
    /*
     * The command is a built-in.
     * Break the command up into arguments and expand wildcards.
     */
    if (!makeArgs(cmd, &argc, &argv, bo, 0, 0)) {
        return true;
    }

    /*
     * Give a usage string if the number of arguments is too large
     * or too small.
     */
    if ((argc < entry->minArgs) || (argc > entry->maxArgs)) {
        fprintf(stderr, "usage: %s %s\n", entry->name, entry->usage);
        return true;
    }

    /*
     * Call the built-in function with the argument list.
     */
    entry->func(argc, argv);
    return true;
}


/*
 * Parse and execute one null-terminated command line string.
 * This breaks the command line up into words, checks to see if the
 * command is an alias, and expands wildcards.
 */
int command(const char *cmd) {
    const char *endCmd;
    char cmdName[CMD_LEN];
    freeChunks();

    /*
     * Skip leading blanks.
     */
    if (cmd) {
        while (isBlank(*cmd))
            cmd++;
        /*
         * If the command is empty or is a comment then ignore it.
         */
        if (cmd) if ((*cmd == '\0') || (*cmd == '#'))
            return 0;
        /*
         * Look for the end of the command name and then copy the
         * command name to a buffer so we can null terminate it.
         */
        endCmd = cmd;
        if (endCmd)
            while (*endCmd && !isBlank(*endCmd))
                endCmd++;

        memcpy(cmdName, cmd, endCmd - cmd);
        cmdName[endCmd - cmd] = '\0';
        /*
         * Expand simple environment variables
         */


        if (cmd)
            while (strstr(cmd, "$("))
                expandVariable((char *) cmd);
        /*
          * Now look for the command in the builtin table, and execute
          * the command if found.
          */
        if (exec_builtin(cmd)) {
            return 0;
        }
        /*
         * The command is not a built-in, so run the program along
         * the PATH list.
         */
        return runCmd(cmd);
    }
    else return 0;
}

void getPath() {
    if (getenv("PATH") == NULL) {
        printf("'%s' is not set.\n", "PATH");
        /* Default our path if it is not set. */
        putenv("PATH=/bin:/usr/bin:/sbin:/usr/sbin:/etc");
    }
    else if (getenv("PATH")) {
        printf("'%s' is set to %s.\n", "PATH", (getenv("PATH")));
    }
}

char *prompt(EditLine *e) {
    static char p2[] = "\1\033[36m$ \033[0m\1";
    static char p[] = "$ ";
    return p;
}

int main(int argc, char *argv[]) {
    struct sigaction sh;
    /* char *shell_prompt[100];*/
    sh.sa_handler = handler;
    sigemptyset(&sh.sa_mask);
    sh.sa_flags = 0;
    sigaction(SIGINT, &sh, NULL);
    int index = 0;
    int i;
    /*  EditLine *el = el_init(argv[0], stdin, stdout, stderr);
      el_set(el, EL_PROMPT_ESC, &prompt, '\1');
      el_set(el, EL_EDITOR, "emacs");
      el_set(el, EL_BIND, "bind ^I el_complete");*/
    rl_bind_key('\t', rl_complete);
    /*rl_parse_and_bind("bind ^I rl_complete");*/

    /*rl_bind_key('\t', rl_complete);*/
    HistEvent ev;
    History *myhistory;
    while (1) {
        index = 0;
        i = getopt_long(argc, argv, "p:vh",
                        options, &index);
        if (i == -1)
            break;
        switch (i) {
            case 'p': {
                /* store_parameter(optarg); */
                break;
            }
            case 'v': {
                printf("OpenShell version 0.1(a)\n");
                printf("Version: %s\n", VERSION);
                exit(EXIT_SUCCESS);

            }
            case 'h': {
                printf("Usage: ./shell\n");
                /*print_help();*/
                exit(EXIT_SUCCESS);

            }
            default: {
                /* fprintf(stderr, "Error (%s): unrecognized option.\n", __FUNCTION__);*/
                /* print_help();*/
                return 1;/*RETURN_FAILURE;*/

            }
        }
    }
    getPath();

    char *shell_prompt;
    for (; ;) {
        shell_prompt = malloc(sizeof(char *) * 1024);
        snprintf(shell_prompt, sizeof(shell_prompt), "%s: $ ", getenv("USER"));
        input = readline(shell_prompt);
        if (input)
            add_history(input);
        command(input);
        free(input);
        free(shell_prompt);

    }
    return 0;
}

util.c

#define _XOPEN_SOURCE 500

#include <sys/stat.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <sys/wait.h>
#include <stdlib.h>
#include "errors.h"
#include <errno.h>
#include <assert.h>
#include "openshell.h"

/*
struct command {
    char *const *argv;
};*/
static CHUNK *chunkList;


/*
 * Free all chunks of memory that had been allocated since the last
 * call to this routine.
 */
void freeChunks(void) {
    CHUNK *chunk;

    while (chunkList) {
        chunk = chunkList;
        chunkList = chunk->next;
        free((char *) chunk);
    }
}

/*
 * Allocate a chunk of memory (like malloc).
 * The difference, though, is that the memory allocated is put on a
 * list of chunks which can be freed all at one time.  You CAN NOT free
 * an individual chunk.
 */
char *getChunk(int size) {
    CHUNK *chunk;

    if (size < CHUNK_INIT_SIZE)
        size = CHUNK_INIT_SIZE;

    chunk = (CHUNK *) malloc(size + sizeof(CHUNK) - CHUNK_INIT_SIZE);

    if (chunk == NULL)
        return NULL;

    chunk->next = chunkList;
    chunkList = chunk;

    return chunk->data;
}

bool find_less_program(char *path) {

    bool found = false;
    char *curr_path;
    const char program[] = "/less";

    while (path && !found) {
        if ((curr_path = malloc(strlen(path) + sizeof(program))) != NULL) {

            strcpy(curr_path, path);
            strcat(curr_path, program);
            if (file_exist(curr_path)) {
                found = true; // we found the program
            }
            free(curr_path);
            path = strtok(NULL, ":");
        } else {
            fprintf(stderr, "malloc failed!\n");
            return false;
        }
    }
    return found;
}

/* Sort routine for list of fileNames.
*/
int nameSort(const void *p1, const void *p2) {
    const char **s1;
    const char **s2;

    s1 = (const char **) p1;
    s2 = (const char **) p2;

    return strcmp(*s1, *s2);
}


/* Routine to see if a text string is matched by a wildcard pattern.
* Returns true if the text is matched, or false if it is not matched
* or if the pattern is invalid.
*  *        matches zero or more characters
*  ?        matches a single character
*  [abc]    matches 'a', 'b' or 'c'
*  \c       quotes character c
*  Adapted from code written by Ingo Wilken.
*/
bool
match(const char *text, const char *pattern) {
    const char *retryPat;
    const char *retryText;
    int ch;
    bool found;

    retryPat = NULL;
    retryText = NULL;

    while (*text || *pattern) {
        ch = *pattern++;

        switch (ch) {
            case '*':
                retryPat = pattern;
                retryText = text;
                break;

            case '[':
                found = 0;

                while ((ch = *pattern++) != ']') {
                    if (ch == '\\')
                        ch = *pattern++;

                    if (ch == '\0')
                        return 0;

                    if (*text == ch)
                        found = 1;
                }

                if (!found) {
                    pattern = retryPat;
                    text = ++retryText;
                }

                /* fall into next case */

            case '?':
                if (*text++ == '\0')
                    return 0;

                break;

            case '\\':
                ch = *pattern++;

                if (ch == '\0')
                    return 0;

                /* fall into next case */

            default:
                if (*text == ch) {
                    if (*text)
                        text++;
                    break;
                }

                if (*text) {
                    pattern = retryPat;
                    text = ++retryText;
                    break;
                }

                return 0;
        }

        if (pattern == NULL)
            return 0;
    }

    return 1;
}

/* This will replace all occurrence of "str" with "rep" in "src"... */
void strreplace(char *src, char *str, char *rep) {
    char *p = strstr(src, str);
    do {
        if (p) {
            char buf[1024];
            memset(buf, '\0', strlen(buf));

            if (src == p) {
                strcpy(buf, rep);
                strcat(buf, p + strlen(str));
            }
            else {
                strncpy(buf, src, strlen(src) - strlen(p));
                strcat(buf, rep);
                strcat(buf, p + strlen(str));
            }

            memset(src, '\0', strlen(src));
            strcpy(src, buf);
        }

    } while (p && (p = strstr(src, str)));
}

char **str_split(char *a[], char *a_str, const char a_delim) {
    char **result = 0;
    size_t count = 0;
    char *tmp = a_str;
    char *last_comma = 0;
    char delim[2];
    delim[0] = a_delim;
    delim[1] = 0;

    /* Count how many elements will be extracted. */
    while (*tmp) {
        if (a_delim == *tmp) {
            count++;
            last_comma = tmp;
        }
        tmp++;
    }

    /* Add space for trailing token. */
    count += last_comma < (a_str + strlen(a_str) - 1);

    /* Add space for terminating null string so caller
       knows where the list of returned strings ends. */
    count++;

    result = malloc(sizeof(char *) * count);
    if (result == NULL) {
        printf("Error allocating memory!\n"); //print an error message
        return result; //return with failure
    }

    if (result) {
        size_t idx = 0;
        char *token = strtok(a_str, delim);

        while (token) {
            assert(idx < count);
            *(result + idx++) = strdup(token);
            token = strtok(0, delim);
        }
        assert(idx == count - 1);
        *(result + idx) = 0;
    }

    return result;
}

/* Expand the wildcards in a fileName wildcard pattern, if any.
* Returns an argument list with matching fileNames in sorted order.
* The expanded names are stored in memory chunks which can later all
* be freed at once.  The returned list is only valid until the next
        * call or until the next command.  Returns zero if the name is not a
        * wildcard, or returns the count of matched files if the name is a
* wildcard and there was at least one match, or returns -1 if either
        * no fileNames matched or there was an allocation error.
*/
int expandWildCards(const char *fileNamePattern, const char ***retFileTable) {
    const char *last;
    const char *cp1;
    const char *cp2;
    const char *cp3;
    const char *possible_tilde;
    char *str;
    DIR *dirp;
    struct dirent *dp;
    unsigned long dirLen;
    int newFileTableSize;
    char **newFileTable;
    char dirName[PATH_LEN];
    char *path;
    static int fileCount;
    static int fileTableSize;
    static char **fileTable;

    /*
     * Clear the return values until we know their final values.
     */
    fileCount = 0;
    *retFileTable = NULL;

    /*
     * Scan the file name pattern for any wildcard characters.
     */
    cp1 = strchr(fileNamePattern, '*');
    cp2 = strchr(fileNamePattern, '?');
    cp3 = strchr(fileNamePattern, '[');

    /*
 * Scan the file name pattern for tilde
 */
    possible_tilde = strchr(fileNamePattern, '~');
    if (possible_tilde != NULL) {
        path = getenv("HOME");
        if (path == NULL) {
            fprintf(stderr, "No HOME environment variable\n");
            return 1;
        }
        strreplace((char *) fileNamePattern, "~", path);
    }

    /*
     * If there are no wildcard characters then return zero to
     * indicate that there was actually no wildcard pattern.
     */
    if ((cp1 == NULL) && (cp2 == NULL) && (cp3 == NULL) && (possible_tilde == NULL))
        return 0;

    /*
     * There are wildcards in the specified filename.
     * Get the last component of the file name.
     */
    last = strrchr(fileNamePattern, '/');

    if (last)
        last++;
    else
        last = fileNamePattern;

    /*
     * If any wildcards were found before the last filename component
     * then return an error.
     */
    if ((cp1 && (cp1 < last)) || (cp2 && (cp2 < last)) ||
        (cp3 && (cp3 < last))) {
        fprintf(stderr,
                "Wildcards only implemented for last file name component\n");

        return -1;
    }

    /*
     * Assume at first that we are scanning the current directory.
     */
    dirName[0] = '.';
    dirName[1] = '\0';

    /*
     * If there was a directory given as part of the file name then
     * copy it and null terminate it.
     */
    if (last != fileNamePattern) {
        memcpy(dirName, fileNamePattern, last - fileNamePattern);
        dirName[last - fileNamePattern - 1] = '\0';

        if (dirName[0] == '\0') {
            dirName[0] = '/';
            dirName[1] = '\0';
        }
    }

    /*
     * Open the directory containing the files to be checked.
     */
    dirp = opendir(dirName);

    if (dirp == NULL) {
        perror(dirName);
        return -1;
    }

    /*
     * Prepare the directory name for use in making full path names.
     */
    dirLen = strlen(dirName);

    if (last == fileNamePattern) {
        dirLen = 0;
        dirName[0] = '\0';
    }
    else if (dirName[dirLen - 1] != '/') {
        dirName[dirLen++] = '/';
        dirName[dirLen] = '\0';
    }

    /*
     * Find all of the files in the directory and check them against
     * the wildcard pattern.
     */
    while ((dp = readdir(dirp)) != NULL) {
        /*
         * Skip the current and parent directories.
         */
        if ((strcmp(dp->d_name, ".") == 0) ||
            (strcmp(dp->d_name, "..") == 0)) {
            continue;
        }

        /*
         * If the file name doesn't match the pattern then skip it.
         */
        if (!match(dp->d_name, last))
            continue;

        /*
         * This file name is selected.
         * See if we need to reallocate the file name table.
         */
        if (fileCount >= fileTableSize) {
            /*
             * Increment the file table size and reallocate it.
             */
            newFileTableSize = fileTableSize + EXPAND_ALLOC;

            newFileTable = (char **) realloc((char *) fileTable,
                                             (newFileTableSize * sizeof(char *)));

            if (newFileTable == NULL) {
                fprintf(stderr, "Cannot allocate file list\n");
                closedir(dirp);
                return -1;
            }
            fileTable = newFileTable;
            fileTableSize = newFileTableSize;
        }

        /*
         * Allocate space for storing the file name in a chunk.
         */
        str = getChunk((int) dirLen + (int) strlen(dp->d_name) + 1);

        if (str == NULL) {
            fprintf(stderr, "No memory for file name\n");
            closedir(dirp);
            return -1;
        }

        /*
         * Save the file name in the chunk.
         */
        if (dirLen) {
            memcpy(str, dirName, dirLen);
        }
        strcpy(str + dirLen, dp->d_name);
        /*
             * Save the allocated file name into the file table.
             */
        fileTable[fileCount++] = str;

    }
    /*
     * Close the directory and check for any matches.
     */
    closedir(dirp);

    if (fileCount == 0) {
        fprintf(stderr, "No matches\n");

        /*  for(int i=0;i<fileCount;i++) {
              printf("fileTable %d %s", fileCount, fileTable[i]);
          }*/
        return -1;
    }

    /*
     * Sort the list of file names.
     */
    qsort((void *) fileTable, (size_t) fileCount, sizeof(char *), nameSort);

    /*
     * Return the file list and count.
     */
    *retFileTable = (const char **) fileTable;

    return fileCount;
}

int do_checkenv(int argc, const char **argv) {
    int status;
    int len = 1;
    char *grep[4];
    char *tmp;
    int k;
    char *pagerValue;
    int pos = 0;
    int i = 0;
    struct command shellcommand[4];
    char *pager_cmd[] = {"less", 0};
    char *printenv[] = {"printenv", 0};
    char *sort[] = {"sort", 0};
    char *path_strdup;
    char *path_value;
    char *pathValue;
    pid_t pid;
    pathValue = getenv("PATH");
    path_strdup = strdup(pathValue);
    path_value = strtok(path_strdup, ":");
    if (find_less_program(path_value)) {
        pager_cmd[0] = "less";
    }
    pagerValue = getenv("PAGER");
    if (!pagerValue) {
        if (find_less_program(path_value)) {
            pager_cmd[0] = "less";
        } else {
            pager_cmd[0] = "more";
        }
    }
    else {
        pager_cmd[0] = pagerValue;
    }

    if (i == 1) {
        /* do nothing */
    }
    else {
        for (k = 1; k < i; k++) {
            len += strlen(argv[k]) + 2;
        }
        tmp = (char *) malloc(len);
        tmp[0] = '\0';
        for (k = 1; k < argc; k++) {
            pos += sprintf(tmp + pos, "%s%s", (k == 1 ? "" : "|"), argv[k]);
        }
        printf("tmp %s", tmp);
        grep[0] = "grep";
        grep[1] = "-E";
        grep[2] = tmp;
        grep[3] = NULL;
        shellcommand[0].argv = printenv;
        shellcommand[1].argv = grep;
        shellcommand[2].argv = sort;
        shellcommand[3].argv = pager_cmd;
        fflush(NULL);
        pid = fork();

        if (pid < 0) {
            perror("fork failed");

            return -1;
        }

        if (pid == 0) {
            fork_pipes(4, shellcommand);

        }
        /*
            * We are the parent process.
            * Wait for the child to complete.
            */
        status = 0;

        while (((pid = waitpid(pid, &status, 0)) < 0) && (errno == EINTR));

        if (pid < 0) {
            fprintf(stderr, "Error from waitpid: %s", strerror(errno));

            return -1;
        }

        if (WIFSIGNALED(status)) {
            fprintf(stderr, "pid %ld: killed by signal %d\n",
                    (long) pid, WTERMSIG(status));

            return -1;
        }

        return WEXITSTATUS(status);

    }
    return 1;
}

int do_editenv(int argc, const char **argv) {
    int r = 0;

    if (getpid() == 1) {
        fprintf(stderr, "You are the INIT process!\n");

        return 1;
    }

    if (argc == 2) {
        r = atoi(argv[1]);
    }

    exit(r);

    return 1;
}

int do_cd(int argc, const char **argv) {
    const char *path;
    if (argc > 1) {
        path = argv[1];
    }
    else {
        path = getenv("HOME");
        if (path == NULL) {
            fprintf(stderr, "No HOME environment variable\n");
            return 1;
        }
    }
    if (chdir(path) < 0) {
        perror(path);
        return 1;
    }
    return 0;
}

int do_exit(int argc, const char **argv) {
    int r = 0;

    if (getpid() == 1) {
        fprintf(stderr, "You are the INIT process!\n");

        return 1;
    }

    if (argc == 2) {
        r = atoi(argv[1]);
    }

    exit(r);

    return 1;
}


int do_killport(int argc, const char **argv) {

    return 0;
}

/*
 * Take a command string and break it up into an argc, argv list while
 * handling quoting and wildcards.  The returned argument list and
 * strings are in static memory, and so are overwritten on each call.
 * The argument list is ended with a NULL pointer for convenience.
 * Returns true if successful, or false on an error with a message
 * already output.
 */
bool makeArgs(const char *cmd, int *retArgc, const char ***retArgv, bool pipe, int g, int h) {
    const char *argument;
    char *cp;
    char *cpOut;
    char *newStrings;
    const char **fileTable;
    const char **newArgTable;
    int newArgTableSize;
    int fileCount;
    int len;
    int ch;
    int quote;
    bool quotedWildCards;
    bool unquotedWildCards;
    bool tilde;
    static int stringsLength;
    static char *strings;
    static int argCount;
    static int argTableSize;
    static const char **argTable;
    /*
     * Clear the returned values until we know them.
     */
    argCount = 0;
    *retArgc = 0;
    *retArgv = NULL;
    tilde = false;
    /*
     * Copy the command string into a buffer that we can modify,
     * reallocating it if necessary.
     */
    len = strlen(cmd) + 1;

    if (len > stringsLength) {
        newStrings = realloc(strings, len);

        if (newStrings == NULL) {
            fprintf(stderr, "Cannot allocate string\n");

            return false;
        }

        strings = newStrings;
        stringsLength = len;
    }

    memcpy(strings, cmd, len);
    cp = strings;

    /*
     * Keep parsing the command string as long as there are any
     * arguments left.
     */
    while (*cp) {
        /*
         * Save the beginning of this argument.
         */
        argument = cp;
        cpOut = cp;

        /*
         * Reset quoting and wildcarding for this argument.
         */
        quote = '\0';
        quotedWildCards = false;
        unquotedWildCards = false;

        /*
         * Loop over the string collecting the next argument while
         * looking for quoted strings or quoted characters, and
         * remembering whether there are any wildcard characters
         * in the argument.
         */
        while (*cp) {
            ch = *cp++;
            /*       if (ch == '|') {
                       *cpOut++ = '|';
                       printf("** deal with it **%s", argument);
                       char *tmpchar = malloc(sizeof(argument));
                       strcpy(tmpchar, argument);
                       char *command[40];
                       tokens = cp;
       */

            /*++tokens = "\0";*
           ;//str_split(command, tmpchar, '|');*/
            /*              calc = 1;
                          continue;*
                      }*/

            /*
             * If we are not in a quote and we see a blank or a pipeline char then
             * this argument is done.
             */
            if (isBlank(ch) && (quote == '\0'))

                break;


            /* check for tilde */

            if (ch == '~') {
                tilde = true;
            }

            /*
             * If we see a backslash then accept the next
             * character no matter what it is.
             */
            if (ch == '\\') {
                ch = *cp++;

                /*
                 * Make sure there is a next character.
                 */
                if (ch == '\0') {
                    fprintf(stderr,
                            "Bad quoted character\n");

                    return false;
                }

                /*
                 * Remember whether the quoted character
                 * is a wildcard.
                 */
                if (isWildCard(ch))
                    quotedWildCards = true;

                *cpOut++ = ch;

                continue;
            }

            /*
             * If we see one of the wildcard characters then
             * remember whether it was seen inside or outside
             * of quotes.
             */
            if (isWildCard(ch)) {
                if (quote)
                    quotedWildCards = true;
                else
                    unquotedWildCards = true;
            }

            /*
             * If we were in a quote and we saw the same quote
             * character again then the quote is done.
             */
            if (ch == quote) {
                quote = '\0';

                continue;
            }

            /*
             * If we weren't in a quote and we see either type
             * of quote character, then remember that we are
             * now inside of a quote.
             */
            if ((quote == '\0') && ((ch == '\'') || (ch == '"'))) {
                quote = ch;

                continue;
            }

            /*
             * Store the character.
             */
            *cpOut++ = ch;
        }

        /*
         * Make sure that quoting is terminated properly.
         */
        if (quote) {
            fprintf(stderr, "Unmatched quote character\n");

            return false;
        }

        /*
         * Null terminate the argument if it had shrunk, and then
         * skip over all blanks to the next argument, nulling them
         * out too.
         */
        if (cp != cpOut)
            *cpOut = '\0';

        while (isBlank(*cp))
            *cp++ = '\0';

        /*
         * If both quoted and unquoted wildcards were used then
         * complain since we don't handle them properly.
         */
        if (quotedWildCards && unquotedWildCards) {
            fprintf(stderr,
                    "Cannot use quoted and unquoted wildcards\n");

            return false;
        }

        if (tilde) {
            /*
             * Expand the argument into the matching filenames.
             */
            fileCount = expandWildCards(argument, &fileTable);

        }

        /*
         * Expand the argument into the matching filenames or accept
         * it as is depending on whether there were any unquoted
         * wildcard characters in it.
         */
        if (unquotedWildCards) {
            /*
             * Expand the argument into the matching filenames.
             */
            fileCount = expandWildCards(argument, &fileTable);

            /*
             * Return an error if the wildcards failed to match.
             */
            if (fileCount < 0)
                return false;

            if (fileCount == 0) {
                fprintf(stderr, "Wildcard expansion error\n");

                return false;
            }
        }
        else {
            /*
             * Set up to only store the argument itself.
             */
            fileTable = &argument;
            fileCount = 1;

        }

        /*
         * Now reallocate the argument table to hold the file name.
         */
        if (argCount + fileCount >= argTableSize) {
            newArgTableSize = argCount + fileCount + 1;

            newArgTable = (const char **) realloc(argTable,
                                                  (sizeof(const char *) * newArgTableSize));

            if (newArgTable == NULL) {
                fprintf(stderr, "No memory for arg list\n");

                return false;
            }

            argTable = newArgTable;
            argTableSize = newArgTableSize;

        }

        if (1) {


/*            for(int i=0;i<argCount;i++){
            }*/

            /*
             * Copy the new arguments to the end of the old ones.
             */

            /*  copy_of_argv = malloc(sizeof(char *) * (argTableSize+fileCount));*/
            /*    memcpy(copy_of_argv,argTable, fileCount*sizeof(const char **));*/

            /*  int q;
              char **copy_of_argv = malloc(sizeof(char *) * (argc-1));
              for (q = 0; q < argc - 1; q++) {
                  copy_of_argv[q] = strdup(argv[q + 1]);
              }
  */

            memcpy((void *) &argTable[argCount], (const void *) fileTable,
                   (sizeof(const char **) * fileCount));

            /*   int i;
               copy_of_argv = malloc(sizeof(char *) * (argTableSize-1));
               for (i = 0; i < argCount - 1; i++) {
                   copy_of_argv[i] = strdup(argTable[i + 1]);
               }
   */

            /*  if (tokens) {
                  int count = argCount + 1;
                  memcpy((void *) &argTable[argCount], (const void *) tokens,
                         (sizeof(const char **) * 20));
                  tokens = NULL;


              }*/

            /*
         * Add to the argument count.
         */
            argCount += fileCount;
        }
    }

    /*
     * Null terminate the argument list and return it.
     */
    if (tilde) --argCount;

    argTable[argCount] = NULL;
    *retArgc = argCount;
    *retArgv = argTable;

    return true;
}
/*

int isBetweenQuotes(int pos, char *str) {
    return IBQplain(pos, str, 0);
}
int IBQsingle(int pos, char *str, int offset) {
    int escaped = 0;
    for (; str[offset]; ++offset) {
        if (!escaped) {
            switch (str[offset]) {
                case '\\':
                    escaped = 1;
                case '\'':
                    return IBQplain(pos, str, offset + 1);
            }
        } else {
            escaped = 0;
        }
        if (pos == offset) {
            return 1;
        }
    }
}

int IBQdouble(int pos, char *str, int offset) {
    int escaped = 0;
    for (; str[offset]; ++offset) {
        if (!escaped) {
            switch (str[offset]) {
                case '\\':
                    escaped = 1;
                case '"':
                    return IBQdouble(pos, str, offset + 1);
            }
        } else {
            escaped = 0;
        }
        if (pos == offset) {
            return 1;
        }
    }
}

int IBQplain(int pos, char *str, int offset) {
    char ch;
    if (pos == offset)
        return 0;  // Not within quotes
    int escaped = 0;
    for (ch = str[offset]; ch; ch = str[++offset]) {
        if (!escaped) {
            switch (str[offset]) {
                '\'': return IBQsingle(pos, str, offset + 1);
                '"':  return IBQdouble(pos, str, offset + 1);
                '\\': escaped = 1
            } else {
                escaped = 0;
            }
            if (pos == offset)
                return escaped;  // Not within quotes, but may be single-escaped
        }
    }
}
*/
/* Helper function that spawns processes */
int spawn_proc(int in, int out, struct command *cmd) {
    pid_t pid;
    fflush(NULL);
    pid = fork();
    if (pid == 0) {
        if (in != 0) {
            if (dup2(in, 0) < 0)
                err_syserr("dup2() failed on stdin for %s: ", cmd->argv[0]);
            close(in);
        }
        if (out != 1) {
            if (dup2(out, 1) < 0)
                err_syserr("dup2() failed on stdout for %s: ", cmd->argv[0]);
            close(out);
        }
        fprintf(stderr, "%d: executing %s\n", (int) getpid(), cmd->argv[0]);
        execvp(cmd->argv[0], cmd->argv);
        err_syserr("failed to execute %s: ", cmd->argv[0]);
    }
    else if (pid < 0) {
        err_syserr("fork failed: ");
    } else {
        /* printf("** we are the parent ***"); */
    }
    return pid;
}

/* Helper function that forks pipes */
void fork_pipes(int n, struct command *cmd) {
    int i;
    int in = 0;
    int fd[2];

    for (i = 0; i < n - 1; ++i) {

        if (pipe(fd) == -1) {
            err_syserr("Failed creating pipe");
        }

        spawn_proc(in, fd[1], cmd + i);
        close(fd[1]);
        in = fd[0];
    }
    if (dup2(in, 0) < 0) {
        err_syserr("dup2() failed on stdin for %s: ", cmd[i].argv[0]);
    }
    fprintf(stderr, "%d: executing %s\n", (int) getpid(), cmd[i].argv[0]);
    execvp(cmd[i].argv[0], cmd[i].argv);
    err_syserr("failed to execute %s: ", cmd[i].argv[0]);
}

/* helper function that determines whether a file exists */
int file_exist(char *filename) {
    struct stat buffer;
    return (stat(filename, &buffer) == 0);
}

void handle_sigchld(int sig) {
    int saved_errno = errno;
    while (waitpid((pid_t) (-1), 0, WNOHANG) > 0) { }
    errno = saved_errno;
}
\$\endgroup\$
3
\$\begingroup\$

Start with your runCmd method, that is one of the most unreadable methods I have ever seen and it CRIES to be broken up into several methods at least. Start with giving each variable a meaningful name, then break up as much as you can into sub methods to reduce the depth. As a general guideline, two nested control flow statements are already hard to read.

\$\endgroup\$
  • \$\begingroup\$ I think I'm going to break out the for loops to become their own functions. I agree it is unreadable. \$\endgroup\$ – Niklas Rosencrantz Apr 30 '16 at 11:28
3
\$\begingroup\$

Method decomposition

The runCmd function starts with ~30 lines of variable declarations. This is a clear sign that the function is doing too many things and need to be split to smaller functions.

Magic numbers

What is number 75 in this code?

for (int x = 0; x < 75; x++) { /* for each pipeline */
    for (int c = 0; c < 75; c++) { /* for each pipeline */
        string[x][c] = '\0';
    }
}

It's really not clear what this is and why it has this specific value. A good way to clarify that is to put in a variable or macro definition with a descriptive name.

Pointless code

This block of code seems completely pointless:

    for (cp = cmd; *cp; cp++) {
        if ((*cp >= 'a') && (*cp <= 'z')) {
            continue;
        }
        if ((*cp >= 'A') && (*cp <= 'Z')) {
            continue;
        }
        if (isDecimal(*cp)) {
            continue;
        }
        if (isBlank(*cp)) {
            continue;
        }
        if ((*cp == '.') || (*cp == '/') || (*cp == '-') ||
            (*cp == '+') || (*cp == '=') || (*cp == '_') ||
            (*cp == ':') || (*cp == ',') || (*cp == '\'') ||
            (*cp == '"')) {
            continue;
        }
    }

Did you forget to implement something inside that loop after all the conditions?


What is going on here?

void handler(int signum) {
    input = '\0';
    if (write(fileno(stdin), s, sizeof s - 1)) { } else { }
    if (signum) { if (false); } else { }
}

input is a global variable. Why update its value here? At the minimum a comment would be nice explaining the reason. Ideally you should avoid global variables altogether.

If we remove the code that does nothing, the function becomes:

void handler(int signum) {
    input = '\0';
    write(fileno(stdin), s, sizeof s - 1));
}

In this code, the variable bo is pointless, it doesn't have a name that tells the reader anything, so you can just as well use its value directly inside the makeArgs call.

bool bo = false;
/*
 * The command is a built-in.
 * Break the command up into arguments and expand wildcards.
 */
if (!makeArgs(cmd, &argc, &argv, bo, 0, 0)) {
    return true;
}

Simplify boolean expression

Instead of this:

return (str && *str && str[strlen(str) - 1] == '}') ? 1 : 0;

You can write simply:

return str && *str && str[strlen(str) - 1] == '}';

And change the return type of the function from int to bool.

Use braces always

Instead of this:

if (endCmd)
    while (*endCmd && !isBlank(*endCmd))
        endCmd++;

The recommended writing style is to always use braces:

if (endCmd) {
    while (*endCmd && !isBlank(*endCmd)) {
        endCmd++;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.