C: read (text) file into array, can't see what's wrong?

Question

first time posting here. I have looked at a few other peoples ways to do this, and one of the ways was nearly exactly the same way that I am trying to do. But, it doesn't work for me?

#include<stdio.h>
int main()
{

FILE *file;
char buffer[15];
char *text[12];

file = fopen("Text.txt", "r");

if(!file) {
    printf("Failed");
    return 1;
}


int count = 0;
while(fgets(buffer,sizeof buffer, file) != NULL) {
    printf("%s", buffer);
    text[count] = buffer;
    count++;
}   
printf("\n");
for (count=0;count<10;count++) {
    printf("%s\n", text[count]);
}

fclose(file);

return 0;

}

Now on another site (while looking for a solution or help I found this http://www.daniweb.com/software-development/c/threads/316766/storing-string-in-a-array-reading-from-text-file

Where the person has done it the same way as me (apart from obviously slight differences in what they're reading etc).

My text file reads: The Quick Brown Fox Jumps Over The Lazy Dog (all on their own lines).

Basically I want to read the file in line by line and save each line in the next space in the array.

In the code when I use the line printf("%s", buffer); It prints out each word of each line okay to the console window. However, when I print out the contents of the array with the for loop it simply prints out "dog, dog, dog..." for each space in the array.

Can someone help me here? Am I doing it wrong?

You did not tell on which system you are coding and against which standard do you code. — Basile Starynkevitch
– Basile Starynkevitch, Commented Jan 26, 2013 at 11:43

P.P · Accepted Answer · 2013-01-26 11:12:50Z

1

You haven't allocated any memory for text. And you have to copy the value of buffer as it changes in each iteration. Something like:

while(fgets(buffer,sizeof buffer, file) != NULL) {
    printf("%s", buffer);
    text[count] = malloc(strlen(buffer) + 1);
    strcpy(text[count], buffer);
    count++;
}

Also, check the return value of malloc for allocation failure.

edited Jan 26, 2013 at 11:12

answered Jan 26, 2013 at 11:03

P.P

122k20 gold badges193 silver badges251 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Basile Starynkevitch Over a year ago

That is wrong, you did not ask malloc for the extra zero ending byte. And using strdup is really simpler and more relevant here.

P.P Over a year ago

Added one char. strdup is simpler but is posix function. May not be available on non *nix systems.

Basile Starynkevitch · Accepted Answer · 2013-01-26 11:42:42Z

1

The buffer is always the same address, so all the elements of array text contains the same pointer (or are uninitialized).

You should consider (assuming a POSIX system):

initializing all your arrays by zeroing them (and also initialize all pointers to NULL), e.g. memset(buffer, 0, sizeof(buffer)); (and for your line buffer, clear it also at the end of the loop).
duplicating the read string with strdup i.e. code
```
text[count] = strdup(buffer);
```
and don't forget to free that when appropriate. Actually, you should check that strdup did not fail by returning a NULL pointer.
use getline to read a line (don't forget to initialize the pointer, perhaps to NULL and to eventually free it).
end your printf formatting string with a newline \n or else call fflush, notably before any input.

use the error code errno to display a better error message. Or just call perror like

file = fopen("Text.txt", "r");
if (!file) {
  perror("fopen Text.txt failed");
  exit (EXIT_FAILURE);
}

Take the habit to compile with all warnings and debugging information (e.g. gcc -Wall -g on Linux) and learn how to use the debugger (e.g. gdb), notably try running line by line your program.

^{You might be interested in coding for a strict (non POSIX) but plain C2011 standard. But then you have much less freedom to use many POSIX functions (like strdup etc...). I leave the boring exercise to code for strict C99 compliance to the reader (remember that C99 does not even know about directories and suppose nearly "flat" filesystems), because I am a Posix and Linux fan and don't care about non Posix systems (in particular I don't care and did not use Windows or Microsoft systems since 1990).}

Also, try to read the code of some free software project (e.g. from freecode). You'll learn a lot

edited Jan 26, 2013 at 11:42

answered Jan 26, 2013 at 11:02

Basile Starynkevitch

1

2 Comments

pmg Over a year ago

-1: Why so many non-standard functions? Couldn't you use malloc() and strcpy() instead of strdup() and fgets() instead of getline()?

Basile Starynkevitch Over a year ago

getline is POSIX so standard. And likewise for strdup. People usually don't code purely ANSI C code. POSIX is a much better standard to code for.

Alexey Frunze · Accepted Answer · 2013-01-26 11:05:35Z

0

There are two problems in the code:

In the while loop you're storing the same pointer (buffer) over and over again into the array of pointers and you are overwriting the buffer contents too. You should probably advance the buffer pointer by the length of each read string.
In the for loop you're destroying the actual line count and potentially dereferencing uninitialized pointers. Don't do that.

answered Jan 26, 2013 at 11:05

Alexey Frunze

62.4k14 gold badges91 silver badges190 bronze badges

1 Comment

user2013417 Over a year ago

I see, I have seen what the other answer above said and I think I know now why that needs to be done!

Collectives™ on Stack Overflow

C: read (text) file into array, can't see what's wrong?

3 Answers 3

2 Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related