fgets loops many times before exiting for EOF

Question

I am making a simple shell. It also needs to be able to read text files by lines. This is my code:

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

// Exit when called, with messages
void my_exit() {
    printf("Bye!\n");
    exit(0);
}

int main(void) {

  setvbuf(stdout, NULL, _IONBF, 0);

  // Char array to store the input
  char buff[1024];

  // For the fork
  int fid;

  // Get all the environment variables
  char dir[50];
  getcwd(dir,50);
  char *user = getenv("USER");
  char *host = getenv("HOST");

  // Issue the prompt here.
  printf("%s@%s:%s> ", user, host, dir);

  // If not EOF, then do stuff!
  while (fgets(buff, 1024, stdin) != NULL) {

    // Get rid of the new line character at the end
    // We will need more of these for special slash cases
    int i = strlen(buff) - 1;
    if (buff[i] == '\n') {
      buff[i] = 0;
    }

    // If the text says 'exit', then exit
    if (!strcmp(buff,"exit")) {
      my_exit();
    }

    // Start forking!
    fid = fork();

    // If fid == 0, then we have the child!
    if (fid == 0) {

      // To keep track of the number of arguments in the buff
      int nargs = 0;

      // This is a messy function we'll have to change. For now,
      // it just counts the number of spaces in the buff and adds
      // one. So (ls -a -l) = 3. AKA 2 spaces + 1. Really in the
      // end, we should be counting the number of chunks in between
      // the spaces.
      for (int i = 0; buff[i] != '\0'; i++) {
        if (buff[i] == ' ') nargs ++;
      }

      // Allocate the space for an array of pointers to args the
      // size of the number of args, plus one for the NULL pointer.
      char **args = malloc((sizeof(char*)*(nargs + 2)));

      // Set the last element to NULL
      args[nargs+1] = NULL;

      // Split string into tokens by space
      char *temp = strtok (buff," ");

      // Copy each token into the array of args
      for (int i = 0; temp != NULL; i++) {
        args[i] = malloc (strlen(temp) + 1);
        strcpy(args[i], temp);
        temp = strtok (NULL, " ");
      }

      // Run the arguments with execvp
      if (execvp(args[0], args)) {
        my_exit();
      }
    }

    //  If fid !=0 then we still have the parent... Need to
    //  add specific errors.
    else {
        wait(NULL);
    }

    // Issue the prompt again.
    printf("%s@%s:%s> ", user, host, dir);
  }

  // If fgets == NULL, then exit!
  my_exit();
  return 0;
}

When I run it alone as a shell, it works great. When I run ./myshell < commands.txt, it does not work.

commands.txt is:

ls -l -a
pwd
ls

But the output is:

>Bye!
>Bye!
>Bye!
>Bye!
>Bye!
>Bye!>Bye!
>Bye!
>Bye!
>Bye!

Doesn't even run my commands. Any ideas? I thought my while loop was pretty simple.

Answer 1

I don't know if this is the problem, but you (correctly) mention in a comment that you have to allocate "plus one for the NULL pointer" in the *args array.

However, you don't actually set the last pointer in *args to NULL.

execvp() won't like that.

That doesn't explain why there might be a difference between redirected vs. non-redirected input, other than undefined behavior is a bastard.

Answer 2

Sorry everyone - turns out my text file was in some sort of demented format from Mac's TextEdit GUI. Everything is working great.

I really appreciate all of the helpful responses