C Fork Usage With User Input

Question

I am trying to write a shell script in C language. In short details, if user enters a command I send it to the system() call, if user enters more than one command like "ls;whoami" I parse it and create child processes to execute all of them. Now it works but my methods such as gets() and getting input by the user does not seem well and when I put multi commands, prompt text becomes unseen. Do you have any suggestion or if you see any mistakes or wrong usage because I am not the C guy then I would be grateful.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#define RESET   "\033[0m"
#define BOLDGREEN   "\033[1m\033[32m"      /* Bold Green */
#define BOLDMAGENTA "\033[1m\033[35m"      /* Bold Magenta */

char input[50];
char command[50];
char *inputget;
char *dotcomma;
char *p;
pid_t pid;

void welcome(){
   char* welcomestr = "\n\TEST\n\n";
   printf("%s%s%s",BOLDMAGENTA,welcomestr,RESET);
}

void prompt(){
   char *username = getenv("USER");
   char hostname[1024];
   gethostname(hostname, 1024);
   char currentDirectory[256];
   getcwd(currentDirectory, 256);
printf("%s%s@%s%s%s ~%s $ %s",BOLDGREEN,username,hostname,RESET,BOLDBLUE,currentDirectory,RESET);
}

int main(int argc, char* argv[]){

    inputget=input;

    welcome();
    prompt();

    gets(inputget);

    if(argc == 1) {

    while(strcmp(inputget, "quit")!=0){
            p = strsep(&inputget, ";\n");

        while(p != NULL){
            pid_t parent = getpid();
            pid = fork();

            if (pid==-1){
                perror("failed to fork");
            }
            else if (pid==0){
                system(p);                                    
                exit(0);       
            }else{  
                p = strsep(&inputget, ";\n");
            }
        }
                    wait(NULL);     

        prompt();
        scanf("%s",input);

        inputget=input;
    }

    exit(0);
    }else{
        //get argc 2 and and read-run commands from text.file
    }
}

Answer 1

Let's begin with the worst part: using gets. Don't use this function anymore, it's dangerous and deprecated. You should use fgets instead.

fgets(input, sizeof input, stdin);

---

There is no reason why any of these variables

char input[50];
char command[50];
char *inputget;
char *dotcomma;
char *p;
pid_t pid;

have to be global variables, declare them in main.

Stream buffers like stdout are buffered, content is physically written on the device once the buffer is full or you call fflush to flush the buffer. One exception is stdout when is connected to a terminal, in that case printf will flush immediately when a newline is printed. That's why you almost always see that the format of printf statements end with \n, like

printf("Your age is %d\n", age);

When you don't want to print a newline, because you are printing something like a prompt, then you should flush stdout yourself.

void prompt(){
    char *username = getenv("USER");
    char hostname[1024];
    gethostname(hostname, 1024);
    char currentDirectory[256];
    getcwd(currentDirectory, 256);
    printf("%s%s@%s%s%s ~%s $ %s",BOLDGREEN,username,hostname,RESET,BOLDBLUE,currentDirectory,RESET);
    fflush(stdout);  // <-- you need this here
}

---

The last thing is where you are executing wait. The problem is the synchronization between the children and the parent process. Once a child is created, it begins to run immediately. If the child is also printing to stdout and you don't synchronize with the parent, then there's no guarantee which output will be printed first.

In your case, if the user enters cmd1;cmd2;cmd3, you are forking 3 times but you are only doing one wait after you've forked all children. That means that the three children will run concurrently and the order of their output is undefined. After all children are forked, you finally do wait(NULL), but this only waits for one child, then you execute prompt() but remember, the other children might be still running and hence the output of the prompt might come before the output of the other children that are running. That is perhaps what you've been observing.

If you want to emulate the shell, then cmd2 can only start after cmd1 is finished and cmd3 only after cmd2 is finished. Only when the three commands are finished you can execute prompt(). That means that you have to wait for every child to end before the next child can be forked. That's why you have to move the wait in the parent block before the next fork is called.

// "quit\n" because fgets does not remove the newline
while(strcmp(inputget, "quit\n") != 0) {
    p = strsep(&inputget, ";\n");

    while(p != NULL) {
        if(p[0] == 0)
        {
            // handles "empty fields", when
            // two delimiters come one after
            // the other 
            p = strsep(&inputget, ";\n");
            continue;
        }

        pid = fork();

        if (pid==-1) {
            perror("failed to fork");
        }
        else if (pid==0) {
            system(p);
            exit(0);
        } else {
            wait(NULL);  // <-- here the parent waits
                         //     until the child is finished
            p = strsep(&inputget, ";\n");
        }
    }

    prompt();
    fgets(input, sizeof input, stdin);

    inputget = input;
}

Also note that I'm not using scanf here. scanf("%s"... reads until the first non-white character, so a command like cat /etc/fstab will only read cat and your shell will only execute cat and it would block until you close stdin (by pressing Ctrl+D). The next time, scanf won't wait for user input and will read /etc/fstab instead and try to execute /etc/fstab, which will fail, as /etc/fstab is not a script or binary. That's why it's better to use fgets.

I'd also use a longer buffer for the user input, depending on the command length, 49 bytes is too short. I'd use 1024 or more. Or you can use getline to fetch a whole line without the worry about buffer sizes.