Implementing a Unix Shell in C

[Enharmonics]

Homework Statement

Assignment instructions are as follows:

You are expected to extend the myshell.c program and add pipelines and I/O redirections. In particular, your shell program should recognize the following:

1. > - Redirect standard output from a command to a file. Note: if the file already exist, it will be erased and overwritten without warning.

Note that you're not supposed to implement the unix commands (ls, sort, ...). You do need to implement the shell that invoke these commands and you need to "wire" up the standard input and output so that they "chain" up as expected.

2. >> - Append standard output from a command to a file if the file exists; if the file does not exist, create one.
3. < - Redirect the standard input to be from a file, rather than the keyboard. For example,

sort < myshell.c

sort < myshell.c > 1

sort > 1 < myshell.c

NOTE: The second and third commands above are the same: the sort program reads the file named myshell.c as standard input, sorts it, and then writes to the standard output to file named 1.

4. | - Pass the standard output of one command to another for further processing. For example,

sort < myshell.c | grep main | cat > output

Make sure you have your parent process to fork the children and 'wire' them up using pipes accordingly.

For the purposes of this assignment, we've been provided with some skeleton code (the "myshell.c" program mentioned in the instructions). I'll post it here:

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

#define MAX_ARGS 20
#define BUFSIZ 1024

int get_args(char* cmdline, char* args[])
{
  int i = 0;

  /* if no args */
  if((args[0] = strtok(cmdline, "\n\t ")) == NULL)
    return 0;

  while((args[++i] = strtok(NULL, "\n\t ")) != NULL) {
    if(i >= MAX_ARGS) {
      printf("Too many arguments!\n");
      exit(1);
    }
  }
  /* the last one is always NULL */
  return i;
}

void execute(char* cmdline)
{
  int pid, async;
  char* args[MAX_ARGS];

  int nargs = get_args(cmdline, args);
  if(nargs <= 0) return;

  if(!strcmp(args[0], "quit") || !strcmp(args[0], "exit")) {
    exit(0);
  }

  /* check if async call */
  if(!strcmp(args[nargs-1], "&")) { async = 1; args[--nargs] = 0; }
  else async = 0;

  pid = fork();
  if(pid == 0) { /* child process */
    execvp(args[0], args);
    /* return only when exec fails */
    perror("exec failed");
    exit(-1);
  } else if(pid > 0) { /* parent process */
    if(!async) waitpid(pid, NULL, 0);
    else printf("this is an async call\n");
  } else { /* error occurred */
    perror("fork failed");
    exit(1);
  }
}

int main (int argc, char* argv [])
{
  char cmdline[BUFSIZ];
 
  for(;;) {
    printf("COP4338$ ");
    if(fgets(cmdline, BUFSIZ, stdin) == NULL) {
      perror("fgets failed");
      exit(1);
    }
    execute(cmdline) ;
  }
  return 0;
}

Homework Equations

N/A.

The Attempt at a Solution

I'll begin by posting my code, which includes the skeleton code provided to us with my own code appended to it, then I'll explain the problem I'm having:

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

#define MAX_ARGS 20
#define BUFSIZE 1024

int get_args(char* cmdline, char* args[])
{
  int i = 0;

  /* if no args */
  if((args[0] = strtok(cmdline, "\n\t ")) == NULL)
    return 0;

  while((args[++i] = strtok(NULL, "\n\t ")) != NULL) {
    if(i >= MAX_ARGS) {
      printf("Too many arguments!\n");
      exit(1);
    }
  }
  /* the last one is always NULL */
  return i;
}

void execute(char* cmdline)
{
  int pid, async, oneapp;
  char* args[MAX_ARGS];
  char* args2[] = {"-l", NULL};
  int nargs = get_args(cmdline, args);
  if(nargs <= 0) return;

  if(!strcmp(args[0], "quit") || !strcmp(args[0], "exit")) {
    exit(0);
  }

    //******************************************************************
    //******************** BEGINNING OF MY CODE ************************
    //******************************************************************
   
  printf("before the if\n");
  printf("%s\n",args[nargs - 2]);
  int i = 0;
  while (args[i] != ">" && i < nargs - 1) {
      printf("%s\n",args[i]);
      i++;
  }

  // Presence of ">" token in args
  // causes errors in execvp() because ">" is not
  // a built-in Unix command, so remove it from args
  args[i - 1] = NULL;
   
  printf("Escaped the while\n");
   
// File descriptor array for the pipe
int fd[2];

// PID for the forked process
pid_t fpid1;

// Open the pipe
pipe(fd);
   
// Here we fork
fpid1 = fork();
   
if (fpid1 < 0)
{
    // The case where the fork fails
   perror("Fork failed!\n");
   exit(-1);
}
else if (fpid1 == 0)
{
       //dup2(fd[1], STDOUT_FILENO);
       close(fd[1]);
       //close(fd[0]);
   
       // File pointer for the file that'll be written to
       FILE * file;
   
       // freopen() redirects stdin to args[nargs - 1],
       // which contains the name of the file we're writing to
       file = freopen(args[nargs - 1], "w+", stdin);
   
       // If we include this line, the functionality works
       //execvp(args[0],args);
   
       // We're done writing to the file, so close it
       fclose(file);
   
       // We're done using the pipe, so close it (unnecessary?)
       //close(fd[1]);
}
else
{
   // Wait for the child process to terminate
   wait(0);
   printf("This is the parent\n");
   
   // Connect write end of pipe (fd[1]) to standard output
   dup2(fd[1], STDOUT_FILENO);
  
   // We don't need the read end, so close it
   close(fd[0]);
  
   // args[0] contains the command "ls", which is
   // what we want to execute
   execvp(args[0], args);
   
   // This is just a test line I was using before to check
   // whether anything was being written to stdout at all
   printf("Exec was here\n");
}
   
// This is here to make sure program execution
// doesn't continue into the original code, which
// currently causes errors due to incomplete functionality
exit(0);

  //******************************************************************
  //*********************** END OF MY CODE ***************************
  //******************************************************************
   
  /* check if async call */
  printf("Async call part\n");
  if(!strcmp(args[nargs-1], "&")) { async = 1; args[--nargs] = 0; }
  else async = 0;

  pid = fork();
  if(pid == 0) { /* child process */
    execvp(args[0], args);
    /* return only when exec fails */
    perror("exec failed");
    exit(-1);
  } else if(pid > 0) { /* parent process */
    if(!async) waitpid(pid, NULL, 0);
    else printf("this is an async call\n");
  } else { /* error occurred */
    perror("fork failed");
    exit(1);
  }
}

int main (int argc, char* argv [])
{
  char cmdline[BUFSIZE];
 
  for(;;) {
    printf("COP4338$ ");
    if(fgets(cmdline, BUFSIZE, stdin) == NULL) {
      perror("fgets failed");
      exit(1);
    }
    execute(cmdline) ;
  }
  return 0;
}

So, what's the problem? Simple: the code above creates a file with the expected name, i.e. the name provided in the command line, which gets placed at args[nargs - 1]. For instance, running the program and then typing

ls > test.txt

Creates a file called test.txt... but it doesn't actually write anything to it. I did manage to get the program to print garbage characters to the file more than a few times, but this only happened during bouts of desperate hail mary coding where I was basically just trying to get the program to write SOMETHING to the file.

I do think I've managed to narrow down the cause of the problems to this area of the code:

else if (fpid1 == 0)
{
       printf("This is the child.\n");

       //dup2(fd[1], STDOUT_FILENO);
       close(fd[1]);
       //close(fd[0]);
  
       // File pointer for the file that'll be written to
       FILE * file;
  
       // freopen() redirects stdin to args[nargs - 1],
       // which contains the name of the file we're writing to
       file = freopen(args[nargs - 1], "w+", stdout);
  
       // If we include this line, the functionality works
       //execvp(args[0],args);
  
       // We're done writing to the file, so close it
       fclose(file);
  
       // We're done using the pipe, so close it (unnecessary?)
       //close(fd[1]);
}
else
{
   // Wait for the child process to terminate
   wait(0);
   printf("This is the parent\n");
  
   // Connect write end of pipe (fd[1]) to standard output
   dup2(fd[1], STDOUT_FILENO);
 
   // We don't need the read end, so close it
   close(fd[0]);
 
   // args[0] contains the command "ls", which is
   // what we want to execute
   execvp(args[0], args);
  
   // This is just a test line I was using before to check
   // whether anything was being written to stdout at all
   printf("Exec was here\n");
}

More specifically, I believe the problem is with the way I'm using (or trying to use) dup2() and the piping functionality. I basically found this out by process of elimination. I spent a few hours commenting things out, moving code around, adding and removing test code, and I've found the following things:

1.) Removing the calls to dup2() and using execvp(args[0], args) prints the result of the ls command to the console. The parent and child processes begin and end properly. So, the calls to execvp() are working properly.

2.) The line

file = freopen(args[nargs - 1], "w+", stdin)

Successfully creates a file with the correct name, so the call to freopen() isn't failing. While this doesn't immediately prove that this function is working properly as it's written now, consider fact #3:

3.) In the child process block, if we make freopen redirect to the output file from stdin (rather than stdout) and uncomment the call to execvp(args[0], args), like so:

       // freopen() redirects stdin to args[nargs - 1],
       // which contains the name of the file we're writing to
       file = freopen(args[nargs - 1], "w+", stdin);
 
       // If we include this line, the functionality works
       execvp(args[0],args);

and run the program, then the program works and result of the ls command is successfully written to the output file. Knowing this, it seems pretty safe to say that freopen() isn't the problem either. (Idea to use freopen() credited to my class's learning assistant who tipped me off about using freopen() to redirect standard input/output directly to a file - I spent the better part of 3 hours trying to find a simple, efficient way to do this)

In other words, the only thing I haven't been able to successfully do is pipe the output of the execvp() call that's done in the parent process to stdout, and then from stdout to the file using freopen().

Any help is appreciated. This assignment is proving extremely difficult for me to get my head around. I've been at this since 10 AM, it's 4 AM the next day as I type this and I'm completely out of ideas. I just don't know what I'm doing wrong.

 

[jedishrfu]

Did you finally work this out?

I’ve done this kind of code in Java but not C where I control the process environment, the process input, the process output and the process error output.

One thing that happens is if you forget to read the process output and error output and thus hang the process. Maybe that is what is happening here.