Why isn't my C shell outputting anything?

Question

Working on a project for a class. We're supposed to write a C shell. I've found a bunch of good examples, but for the life of me, I can't get my version to generate any output.

It keeps printing the prompt, but nothing else.

I've followed along with some examples near-verbatim trying to fix this, but still nothing.

#include <stdio.h>

#include <stdlib.h>

#include <unistd.h>

#include <string.h>

#include <stdbool.h>

#include <sys/wait.h>



/******************************************

  @brief Fork a child to execute the command using execvp. The parent should wait for the child to terminate

  @param args Null terminated list of arguments (including program).

  @return returns 1, to continue execution and 0 to terminate the MyShell prompt.

******************************************/

int execute(char **args)

{

    pid_t  pid;
        int status;

        if ((pid = fork()) < 0) 
        {     
              printf("*** ERROR: forking child process failed\n");
              exit(1);
        }
        else if (pid == 0) 
        {          
          if (execvp(*args, args) < 0) {     
               printf("*** ERROR: exec failed\n");
               exit(1);
          }
        }
        else 
        {                                  
          while (wait(&status) != pid)       
               ;
        }
        
        return 0;

}





/******************************************

  @brief gets the input from the prompt and splits it into tokens. Prepares the arguments for execvp

  @return returns char** args to be used by execvp

******************************************/

char** parse(void)

{

    //Get the string and store it. Remove newline.  

    char strIn[255];

    printf("MyShell>");

    fgets(strIn, sizeof(strIn), stdin); 

    strIn[strlen(strIn)-1]='\0';

    

    //Array, token, counters. 

    char *args[20]; 

    char *tok; 

    int i = 0;

    int count = 0;

    

    //Allocate array. 

    for(i = 0; i < 20; i++)

    {

        args[i] = (char*)malloc(20 * sizeof(char));

    }

    

    //Fill in args[0]

    tok = strtok(strIn, " ");

    strcpy(args[count++], tok);

    

    //Fill in array with tokens. 

    while (tok != NULL)

    {

        tok = strtok(NULL, " ");

        

        if (tok == NULL)

            break;

            

        strcpy(args[count++], tok);

    }

    

    //Null terminate.

    args[count] = NULL;

        

    return args; 

}





/******************************************

   @brief Main function should run infinitely until terminated manually using CTRL+C or typing in the exit command

   It should call the parse() and execute() functions

   @param argc Argument count.

   @param argv Argument vector.

   @return status code

******************************************/

int main(int argc, char **argv)

{

    bool run = true; 

    

    while(run)

    {

        char** argArr = parse(); 

        execute(argArr);


        }

        

    return 0;

}

The output, regardless of what I do, is:

MyShell>
MyShell>
MyShell>

Can someone tell me where I went wrong?

Answer 1

parse() returns a pointer to the local array args. Since the lifetime of args ends when parse() returns, any attempt to use the return value of parse() is undefined behavior. You should allocate that array with malloc() instead (and free it later!).

What happens in my test is that the compiler notices that the return value of parse() can't legally be used (and it gives a warning!! which you should read and pay attention to!!), so it just returns NULL instead. When the child dereferences this pointer as *args to get the first argument for execvp, it segfaults and dies without calling execvp(). You could detect this if you checked the status returned by wait(), but you don't. So it just looks as if the child didn't do anything.

Oh, bonus bug: when end-of-file occurs on stdin (e.g. if you hit Ctrl-D), the string returned by fgets() will be empty and strIn[strlen(strIn)-1]='\0'; will store a null byte out of bounds. You need to test the return value of fgets().