CODEWITHSUNDEEP
cpointers
francescobfc
francescobfc

Reputation: 107

Code to store a file's lines in a char double pointer is storing the same line in all elements

I want to make a code that reads a file and stores each line of it into a string. On the other hand, I store these strings into another array, which makes a char double pointer. The code is printing the file rightly, but when I try to print a string (one of the lines) randomly it always outputs the last line of the file ("{").

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

int main()
{
    //I Open the file
    FILE* file_1 = fopen ("Caso2.ll", "r");
    char string[1000];
    //I create a double pointer to char of size 1000 (big enough to store any string)
    char ** string_2 = (char**) malloc(sizeof(char *)*1000);
    int i =0;
    //While fgets is different from NULL
    while (fgets(string, 1000, file_1)!= NULL)
    {
        //I define each element of string_2 as a char pointer (string)
        string_2[i] = (char *)malloc(sizeof(char)*1000);
        //and attribute string to string_2, thus storing the read line into it
        string_2[i] = string;
        //I print string_2
        printf("%s", string_2[i]);
        i++;
    }
    //Until now everything is okay. The code is printing the file okay.
    printf("\n\n\n\n\n\n\n");
    //Here is where the error is. When I try to print a random element of 
    //string_2, it always prints "{", the last line of the file
    fputs(*(string_2 + 5), stdout);
    system("pause");
    return 0;
}

Upvotes: 1

Views: 619

Answers (2)

user3629249
user3629249

Reputation: 16540

regarding: 

string_2[i] = string;

This always copies (ONLY) a pointer to string so, all the entries in string_2 point to string and string contains what ever was read last.

Suggest replacing that statement with: 

strcpy( string_2[i], string );

regarding:

fputs(*(string_2 + 5), stdout);

when the strcpy() is implemented, then the call to fputs() will not work due to an extra de-reference. suggest:

fputs( string_2 + 5, stdout );

The posted code contains a massive memory leak. To fix this suggest: replace:

string_2 = (char *)malloc(sizeof(char*)*1000);

with

string_2 = calloc( sizeof( char *), 1000 );

so all the pointers in string_2 contain NULL, then insert the following just before the return statement:

for( int i=0; i<MAX_INPUT_LINES; i++ )
{
    free( string_2[i] );
}
free( string_2 );

Note: the function: free() handles a NULL parameter with no problems.

for robust code, regarding this kind of statement:

string_2[i] = (char *)malloc(sizeof(char)*1000);

always check (!=NULL) the returned value to assure the operation was successful.

Note: 1000 is a 'magic' number. To use a meaningful name, suggest:

#define MAX_INPUT_LINES 1000

Then use the name: MAX_INPUT_LINES when calling calloc()

Do a similar name when allocating room for each line from the input file.

The code should stop inputting lines if there is more than 1000 lines in the input file. suggest replacing:

while (fgets(string, 1000, file_1)!= NULL)

with:

while ( i < MAX_INPUT_LINES && fgets(string, 1000, file_1)!= NULL)

Upvotes: 2

Adrian Mole
Adrian Mole

Reputation: 51894

The problem is the way you are assigning data to your string_2 array. In your read loop, you do this:

string_2[i] = string;

Which is assigning to the pointer that is string_2[i] the value of the pointer that is string. So, when you later change the data in string, you will also change the data pointed to by each, previously-assigned, string_2[i] pointer. Thus, when your loop ends, all the string_2[] pointers will point to the (same) string data.

What you should do instead, in your loop, is this:

strcpy(string_2[i], string);

This will copy the current data in string to your indexed and (correctly) allocated new string_2[i] buffer.

Feel free to ask for further clarification and/or explanation.

Upvotes: 1

Related Questions