2

I'm trying to develop a function that reads each line of a text file and the it stores them in an array of strings (char**) but fgets() doesnt seem to work, it always return a null character.

Here is the function

 char** getLines(FILE* fp){

    char** lines;
    int numberOfLines; //number of lines int the file
    char ch; //aux var
    int i; //counter

    while(!feof(fp)){
        ch = fgetc(fp);
        if( ch == '\n'){
            numberOfLines++;
        }
    }

    lines = malloc(numberOfLines*sizeof(char*));

    if (lines==NULL){
        fprintf(stderr,"Error, malloc failed");
        exit(1);
    }

    for(i = 0; i<numberOfLines; i++){
        lines[i] = malloc(MAX_LENGTH*sizeof(char)); //MAX_LENGTH = 128
    }

    i=0;
    while(fgets(lines[i], MAX_LENGTH,fp)){
        printf("Line %d: %s \n",i,lines[i]);
        i++;
    }

    return lines;

}

The function never gets inside the while loop so it doesn't print anything I'm also using a very simple input file:

test line 1
test line 2
test line 3
test line 4

Hope you can help me, Thank you in advance.

Setekorrales
  • 105
  • 2
  • 11
  • did I read `MAX_LENGHT` instead of `MAX_LENGTH` ? – Rahul Verma Oct 05 '17 at 10:58
  • C or C++ - choose one. Your code looks like C. –  Oct 05 '17 at 10:58
  • 2
    I recommend you take some itme to read [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) Also note that [`fgetc`](http://en.cppreference.com/w/c/io/fgetc) returns an `int`. – Some programmer dude Oct 05 '17 at 10:59
  • 1
    As for your problem, think about where the file pointer is after you checked the number of lines. If only there were some way to [*rewind*](http://en.cppreference.com/w/c/io/rewind) the file pointer to the start again... – Some programmer dude Oct 05 '17 at 11:00
  • 1
    You read the entire file before ever calling `fgets`. So of course it's not reading anything. There's nothing left to read. You should fix your code to dynamically grow the line array as needed while reading lines, and only try to read the file once. – Tom Karzes Oct 05 '17 at 11:00
  • @Someprogrammerdude We really need a quick "This is some kind of a dupe of [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong)" close option... – Andrew Henle Oct 05 '17 at 11:01
  • I strongly advise against trying to rewind the file for a second pass. That needlessly restricts your app to files that support seek, and in particular it will fail for sockets (including pipes), console input, etc. Do it the right way, as I described. – Tom Karzes Oct 05 '17 at 11:04

1 Answers1

3

You are already on the end of the file before entering the while loop.

Take a look here http://en.cppreference.com/w/cpp/io/c/rewind

Moves the file position indicator to the beginning of the given file stream. The function is equivalent to std::fseek(stream, 0, SEEK_SET); except that end-of-file and error indicators are cleared. The function drops any effects from previous calls to ungetc.

Check if this works:

char** getLines(FILE* fp){
    /* ...... */
    i=0;
    rewind(fp); // Rewind here
    while(fgets(lines[i], MAX_LENGTH,fp)){
        printf("Line %d: %s \n", i, lines[i]); // Also use the index as first parameter
        i++;
    }

    return lines;

}
Raynigon
  • 660
  • 1
  • 9
  • 21
  • Ok, I see it know, thank you very much, do you know how can I get to the start of the file again? – Setekorrales Oct 05 '17 at 11:02
  • thank you very much, It worked, but now I get a segmentation fault when i try to print the line – Setekorrales Oct 05 '17 at 11:06
  • 3
    Rewinding is a lazy way to do this, and it prevents it from working on streams that don't support seek, such as sockets (including pipes), console input, etc. The right way to do this is to dynamically adjust your array as you read more and more lines. – Tom Karzes Oct 05 '17 at 11:06
  • @SimonSchneider Actually, there is no need to repeat the whole code (just makes spotting the rewind part more difficult...). Better only give some hints of what was before and what comes after (e. g. just leaving the loops immedieately before and after, or only their headers followed by a comment (`/* ... */`). – Aconcagua Oct 05 '17 at 11:07
  • 1
    @TomKarzes thats right, but it looks like there is no need to make it better here. – Raynigon Oct 05 '17 at 11:08
  • @Setekorrales Take a look at your printf you want to print to values, but only one is given. – Raynigon Oct 05 '17 at 11:08
  • @Aconcagua Thanks, i think now it should be more easy to read – Raynigon Oct 05 '17 at 11:12