2

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;
}
5
  • 2
    Can you tell us what the behavior is? Commented Oct 26, 2010 at 2:27
  • Well it runs and loops through back to prompt I just dont think its executing the commands. Commented Oct 26, 2010 at 2:42
  • 2
    First, your formatting an indenting is horribly inconsistent, which makes it difficult to read the code. Second, what do you mean, "what am I doing wrong?"? What doesn't work? What do you expect, and what do you actually get? Fer heck's sake, you don't even tell us if its a compile-time/link-time/run-time error! We do not debug-via-ESP. Commented Oct 26, 2010 at 2:45
  • edit your Question to include more information. Do not put your responses in comments. Commented Oct 26, 2010 at 2:46
  • You probably want execvp(argv[0], &argv[1]); . Commented Oct 26, 2010 at 2:49

2 Answers 2

3

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.

Sign up to request clarification or add additional context in comments.

Comments

0

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.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.