Thread: Reading from file into array, some strings aren't terminated?

I'm making a Jeopardy game and when I read text from a file into arrays to set the categories, questions and answers a couple of the strings are not terminating and they are printing that junk box thing at the end. When I use the debugger and print the problem strings they are showing up as "Thanksgiving\021" or "Calvin Klein\021" but all the other strings aren't having that problem. Then for example I will change "line[strlen(line)-1] = '\0';" to "line[strlen(line)] = '\0';" and other strings will have that problem but not those stated ones. I'm using line[strlen(line) - 1] = '\0' to get rid of the newline fgets appends on the end of the strings. Thanks for the help.

Code:

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


int main(int argc, char *argv[]){
   
   FILE *file = fopen("questions", "r");
   char *string[66], line[150];
   int idx;


   for(idx = 0; idx < 66; idx++){
      fgets(line, 150, file);
      line[strlen(line)-1] = '\0';
      string[idx] = (char *) malloc(strlen(line));
      strcpy(string[idx], line);
   }


   for(idx = 0; idx < 66; idx++){
    
      printf("%s\n", string[idx]);
   }




   return 0;
}

Nevermind, I just figured it out! I wasn't terminating the strings in the array itself.

Code:

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


int main(int argc, char *argv[]){
   
   FILE *file = fopen("questions", "r");
   char *string[66], line[150];
   int idx;


   for(idx = 0; idx < 66; idx++){
      fgets(line, 150, file);
      line[strlen(line)] = '\0';
      string[idx] = (char *) malloc(strlen(line));
      strcpy(string[idx], line);
      string[idx][strlen(string[idx]) - 1] = '\0';
   }


   for(idx = 0; idx < 66; idx++){
    
      printf("%s\n", string[idx]);
   }


   return 0;
}

Originally Posted by Click_here

Also, you are checking for no errors.

You should check to see if file is not equal to NULL before using it
You should check to see if malloc did not return NULL before using it
You should not cast what malloc returns (It's unneccesary and considered bad practice in C, because it will hide an error from your compiler)

That code I posted was just a quick example, in my actual program I checked for a NULL file and used free() for the dynamic memory alloc. But I wasn't aware casting malloc was bad practice, thanks.