Simple Linux Shell - execvp() failing

462 views Asked by At

I need some help with a simple shell for class, and I'm worried I don't quite understand how the execvp() function works.

The shell does not do much, does not support piping, redirection, scripting or anything fancy like that. It only reads a command, reads in the options (with the command as option[0]), and forks.

It worked a few times, then started giving me errors about not being able to find commands. Other similar questions posted here had to do with piping or redirection.

Please forgive the noobcode, it's not pretty, but I hope it's legible:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/types.h>

#define OPT_AMT 10

const size_t SIZE = 256;
int i = 0;
int o = 0;

int main(void) {

    // initializing data

    int exit = 0;
    char cwd[SIZE];
    char cmd[SIZE];
    char input[SIZE * OPT_AMT];
    char *opt[OPT_AMT];
    getcwd(cwd, SIZE);

    // main loop

    while (exit == 0) {
        // reset everything
        o = 1;
        i = 0;
        cmd[0] = "\0";
        while (i < OPT_AMT) {
            opt[i++] = "\0";
        }

        // get input
        printf("%s $ ", cwd);
        scanf("%s", cmd);
        gets(input);
        opt[0] = cmd;

        char *t = strtok(input, " ");
        while (t != NULL) {
            opt[o++] = t;
            t = strtok(NULL, " ");
        }

        // if exit, exit
        if (strcmp(cmd, "exit") == 0) {
            exit = 1;
        }
        // else fork and execute
        else {
            pid_t pID = fork();

            if (pID == 0) { // child process
                execvp(cmd, opt);
            } else if (pID < 0) { // failed to fork
                printf("\nFailed to fork\n");
            } else { // parent process
                wait(0);
            }
        }
    }

    // cleanup

    printf("\nFinished!  Exiting...\n");
    return 0;
}

Anything blatantly wrong? I most recently added the exit condition and the resetting the options array.

Also, this is my first question, so remind me of any rules I may have broken.

1

There are 1 answers

5
alk On BEST ANSWER

For starters, this

cmd[0] = "\0";

ought to be

cmd[0] = '\0';

Listen to your compiler's warnings.

To enable them use the options -Wall -Wextra -pedantic (for gcc).


Also you might better want to initalise opt's elements to point to "nothing", that is NULL but to the literal "\0":

    while (i < OPT_AMT) {
        opt[i++] = NULL;
    }

as execvp() requires opt to be a NULL-terminated array of C-"strings" (thanks Paul for mentioning/wording the relevant background).


Also^2: Do not use gets(), as it's evil and not even part of the C Standard anymore. Instead of

gets(input);

use

fgets(input, sizeof input, stdin);

gets() easyly let's the user overflow the (input) buffer passed. (Mentioning this came to my mind without Paul, btw ... ;-))