Search code examples
cfgetsdynamic-memory-allocation

What will cause fgets() to continuously wait for input?


I am trying to put together a program that will ask the user to enter song titles for a set list to be printed in a random order. The program uses fgets() to take in the song titles. It also uses memory allocation to put each song in. It is similar to:

 argv[0] = song1, argv[1] = song2, argv[2] = song3 (etc.)

The problem I am running into is when the program is executed fgets() waits continuously for input, when it is only a total of five songs to be entered. I want to know what will cause fgets() to continuously wait for input? Thanks for your help.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>


int main( void ){

  printf("\tPlease enter the songs you want to play!\n");
  printf("\tI will create the set list and randomize it for you!\n");
  printf("\t(enter all songs with a space in between each and then press 'enter')\n");


  int s = 5; //Here we declare number of songs to be entered
  int set = 5;//This is set list size
  char input[100];
  char *argv[ s ];
  char *token;

  /* get the input and perform memory allocation */
  fgets(input, s, stdin);

  token = strtok(input, " ");

  int i=0;

  while( token != NULL ) {

    argv[i] = malloc(strlen(token) + 1);
    memcpy(argv[i], token, strlen(token)+1);
    i++;
    token = strtok(NULL, " ");
  } 
  argv[i] = NULL; //argv ends with NULL

  unsigned int setList[ s ];
  memset( setList, 0, s*sizeof(unsigned int) );//Must manually initalize array
  srand( time( NULL ) ); // seed random-number generator

  /*Begin Randomize Code*/
  size_t column;
  size_t c;
  size_t c1;
  size_t column1;

  for ( c = 1; c <= set; ++c ) {
    do {
      column = rand() % s;

    } while( setList[ column ] != 0 );

    setList[ column ] = c;

  }//end of for
  /*End Randomize Code*/

  /*Begin Code to Print SetList*/
  for ( c1 = 1; c1 <= set; ++c1 ) {

    for ( column1 = 0; column1 < s; ++column1 ) {

      if ( setList[ column1 ] == c1 ) {

        printf( "%s\n", argv[ column1 ]); 

      }//end of for  (oops if)
    }//end of for
  }//end of if  (oops for)
  /*End Code to Print SetList*/

}//end of main

Solution

  • Actually, the problem is right here:

    fgets(input, s, stdin); <-- you tell fgets to only read 5 characters (actually only 4, the fifth character is the null terminator); this means that your tokenization will fail and not allocate memory for all five elements of argv, causing an access violation in your printf call later.

    Change it to: fgets(input, sizeof(input), stdin); and you get this:

    http://i.gyazo.com/ec6185438adfb5c56c98ec5fcef67eaf.png

    Some other problems with the code:

    • argv is, by convention, the name of the tokenized command line string passed to your main function, so don't name your variables like this, it's confusing
    • instead of using a variable for things like the maximum size of something, use #define, e.g. #define MAX_SONGS 5
    • it is very buggy and will crash if bad input is given, you should validate it (for instance, if I enter more than 5 songs, you'll overflow your buffer and most likely crash)