Help creating a simple C shell

Question

Hello everyone, I am new to C and I am trying to create a simple C shell that will allow the user to perform various functions like chdir, cd, exit, mkdir.

I've posted my code below. Can anyone look through it and see what I am doing wrong? I am not sure if I am using fork and execcv correctly. Thanks!

include stdio.h
include stdlib.h
include unistd.h
include <string.h>
include sys/types.h

main() {
    //char *user;

    //if ((user = getlogin()) == NULL)
    //    perror("__getlogin1() error");
    //else printf("__getlogin1() returned %s\n", user);
    int j, status;
    int pid, c_pid;
    int i = 0;
    char *tmp, **ap;
    char instring[80]; // store one line of input
    char *argv[10]; // store parameters in the format for execv()

    promptstart:

    printf("Please enter a commcand:\n");

    // read a char at a time and put it in instring[]
    // put a '\0' at the end
    instring[i] = getc(stdin); // stdin is the keyboard
    while (instring[i] != '\n') {
        i++;
        instring[i] = getc(stdin);
    }
    instring[i] = '\0'; // replace '\n' with '\0'

    tmp = instring;
    i = 0;
    argv[i] = strsep(&tmp, " \t"); // put first word int argv[0]
    while ((i < 10) && (argv[i] != '\0')) {
        i++;
        argv[i] = strsep(&tmp, " \t");
    }

    // print out the command and options.
    i = 0;
    while (argv[i] != '\0') {
        printf("your entered: %s\n", argv[i++]);
    }

    //PLACE ERROR HERE

    if ((c_pid = fork()) == 0) {
        for (j = 0; j < 10; j++)
            printf("child (%d) prints %d\n", getpid(), j);
        exit(0);
    } else if (c_pid > 0) {
        c_pid = wait(&status);
        printf("child %d exited with status %d\n", c_pid, status);
    } else {
        execvp(argv[0], argv);

    }
    goto promptstart;
}

Answer 1

At least IMO, you're putting far too much into main. I'd start with something like:

int main() { 
    char input[128];

    do { 
        fgets(stdin, input, sizeof(input));
        dispatch(input);
    } while (strcmp(input, "exit"));
    return 0;
}

Then dispatch will look for internal commands, and only do an exec when/if it's given a command it doesn't recognize. To keep things simple to start with, you might consider using popen to execute external commands, and leave switching to a "raw" fork/exec for later, when the limitations of popen start to cause you problems.

Answer 2

For shell builtins (man bash), you probably don't want to fork/exec. I would save fork/exec for running programs that are in your PATH (an environment variable that your shell will have to manage). The shell itself should interface with the filesystem through commands like chdir (man 2 chdir).

Consider using a nice string tokenizer (or just fallback to strtok) for parsing the commandline, and as another comment suggests, abstract that into a function so that your main loop is lean.