0

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
AShelly
  • 34,686
  • 15
  • 91
  • 152
Raoul Duke
  • 131
  • 7
  • 24
  • If you would start to indent your code properly you would not have to place comments everywhere explaining each closing bracket. Please edit your question and probably your source code as well to following some coding standard. – luk2302 May 28 '15 at 17:18
  • What happens when you enter a list longer than 100 characters? – AShelly May 28 '15 at 17:20
  • 2
    Also, please don't use `argv` for your own variables. – AShelly May 28 '15 at 17:20
  • I now understand how my indentation can complicate things. Thank You. – Raoul Duke May 28 '15 at 17:26
  • 1
    `int s = 5; fgets(input, s, stdin);` will allow user to enter 3 letter input (plus `\n` plus `\0`) Suggest `fgets(input, sizeof input, stdin);` – chux - Reinstate Monica May 28 '15 at 17:33

1 Answers1

3

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)
user4520
  • 3,401
  • 1
  • 27
  • 50