momonosuke
momonosuke

Reputation: 75

Reading string from input file and storing them into a dynamic array

I wrote a Code that reads string input from a file then stores them into a dynamic allocated array.

I believe that I have stored the elements correctly , however when I try to print out those elements I get a segmentation Fault.

int main (int argc, char **argv) {

char reds [13];

int i;

    FILE *file = argc > 1 ? fopen (argv[1], "r") : stdin;
    if (file == NULL)
        return 1;
        if(argc!=2){
            printf("[ERR]");
            return 0;
        }

   for (i =0; i < 13; i++) {


      char *reds = malloc (sizeof(char)); 
    fscanf (file, "%s", &reds[i]);
  }


      for (i =0; i < 13; i++){
    printf (" %s\n", reds[i]);

  }
    return 0;   

   } 

input file:

RED A
RED 2
RED 3
RED 4
RED 5
RED 6
RED 7
RED 8
RED 9
RED 10
RED J
RED Q
RED K

Could someone tell me what I am doing wrong? Thank you in advance.

Upvotes: 0

Views: 842

Answers (1)

TypeIA
TypeIA

Reputation: 17258

char reds [13];

This allocates a fixed-size character array with 13 elements; it can hold a single string up to a length of 12 chars (plus the NULL terminator). That's not what you intended.

char *reds = malloc (sizeof(char));

This creates a new variable reds which hides the one you declared above. This one is a pointer to an array with enough room for only a single char; it can hold only an empty string (the NULL terminator). That's also not what you intended.

fscanf (file, "%s", &reds[i]);

This passes fscanf() a pointer to the i'th element of the (original) reds array. Since reds has 13 elements, that means if the string stored by fscanf() has more than 12 - i characters in any loop iteration, the behavior will be undefined. Again, not what you intended.


What you probably meant to do was something like this:

char *reds[13]; // an array of 13 POINTERS

// later, in your loop...
reds[i] = malloc(sizeof(char) * (max_string_length + 1));
fscanf(file, "%s", reds[i]); // see note below

1) Don't forget to free() the pointers you allocate with malloc() or your program will have a memory leak. In your example it doesn't matter so much because the program exits right away (releasing the memory), but if it didn't that memory would be leaked, so you should develop good habits.

2) It's still possible to invoke undefined behavior, if fscanf() stores a string longer than max_string_length; you can improve this further by passing a maximum width specified to fscanf().

3) sizeof(char) is guaranteed to equal 1, so you can omit this if you want (although there's no harm in leaving it in for clarity if you prefer this).

4) Do error-check the return values of malloc(), fscanf(), fopen() etc. If you don't, and an error occurs, your program can easily go on to invoke undefined behavior, which can be difficult to debug. Save yourself a lot of pain and develop this habit now.

Upvotes: 2

Related Questions