2

I have a similar program that'll read an entire text file no problem. But maybe it's how i have my fscanf here? First time I've run into something like this tbh. Basically I'm storing the first string in a file into a variable, and trying to use that to go through each line in a second file to see if the string is there.

#define MAX_LENGTH 500
#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>



int main() {

        char string1[MAX_LENGTH]; //this will each string from my first file to compare every string in the second file
        char string2[MAX_LENGTH];
        int result;

        FILE *fp1;
        FILE *fp2;
        fp1 = fopen("C:/textfile1.txt", "r");
        fp2 = fopen("C:/textfile2.txt", "r");

        if (fp1 == NULL || fp2 == NULL) {

            printf("FILE cannot be opened... exiting...");
            exit(1);

        }

        //while (1) {

            while(! feof(fp1)) { 

                fscanf(fp1, "%[^\n]s", string1); 

                while (! feof(fp2)) {

                    fscanf(fp2, "%[^\n]s", string2); 
                    result = strcmp(string1, string2);

                    if (result == 1) { 

                        printf("%s has been ADDED...\n", string1);

                    }
                }

            }

            while(! feof(fp2)) {

                fscanf(fp2, "%[^\n]s", string1);

                while (!feof(fp1)) {

                    fscanf(fp1, "%[^\n]s", string1);
                    result = strcmp(string2, string1);

                    if (result == 1) {

                        printf("%s has been REMOVED...\n", string2);
                    }
                }

            }
        //}

        getchar();
        getchar();

        return 0;

}
br34k
  • 57
  • 6
  • 2
    You're expecting `feof` to tell you whether or not a future read will succeed. But you can't use `feof` to predict the future, it only tells you about what has already happened. Instead, check if the read itself succeeded rather than trying to check before the read if it will succeed. – David Schwartz Jan 27 '19 at 01:34
  • Isn't feof used to check if the file has hit the End of File? Should I use a different parameter for my while() loop? – br34k Jan 27 '19 at 01:47
  • 1
    Yes, `feof` is used to check if you've already hit the end of file. But you're acting as if it will tell you whether or not a future read will hit the end of file. You should, as I said, check if the read succeeds and break out of the loop if the read attempt hits the end of the file. As a general rule, it's almost always wrong to try to do some test to determine whether or not a future operation will succeed and then just assume that the future operation succeeded when you actually do it. That's what your code does. Instead, check if the actual operation succeeds. – David Schwartz Jan 27 '19 at 01:49
  • Also, if you want to read a line, I suggest using a function specifically documented to read a line. C has a nice [getline](http://man7.org/linux/man-pages/man3/getline.3.html) function. Be sure to check the return value! – David Schwartz Jan 27 '19 at 01:52
  • 2
    [This question](https://stackoverflow.com/questions/5431941) has more information about `feof`. – user3386109 Jan 27 '19 at 02:38
  • @DavidSchwartz Standard C, unfortunately does not have the nice `getline()`. It is a frequent extension though. – chux - Reinstate Monica Jan 27 '19 at 03:54
  • @br34k Who or what text suggested `while(! feof(fp1)) { fscanf(fp1, "%[^\n]s", string1);`? – chux - Reinstate Monica Jan 27 '19 at 03:58

2 Answers2

1

Firstly, Let me start by saying that fscanf() isn't the ideal function for what you are doing, use fgets() or getline() as others have already stated. Secondly, you should notice that the two last while statements in your program will be skipped since by the time the execution flow reach that point, both the streams fp2 and fp1 will already be on EOF.

Now, concerning you question, your problem is the following pattern: "%[^\n]s" If your intention is to read a line of text, the trailing 's' isn't necessary and it would require the input stream to contain 's' after the sequence matched by %[^\n]

What the %[^\n] pattern means is "read everything up to the first '\n', but not including the '\n' itself, which will be left in the buffer; This is not good, once the next fscanf() call would read this left over '\n' and nothing more. To solve this issue you will need to clean the buffer, something like fscanf(fp, "%*c") should work. It will read in the spare '\n' character, it will be necessary to do that after every single call to fscanf(). Something like so:

        while(! feof(fp1)) { 

            fscanf(fp1, "%[^\n]", string1);
            fscanf(fp1, "%*c");

            printf("%s\n", string1);

            while (! feof(fp2)) {

                fscanf(fp2, "%[^\n]", string2);
                fscanf(fp2, "%*c");

                printf("%s\n", string2);

                result = strcmp(string1, string2);

                if (result == 1) { 

                    printf("%s has been ADDED...\n", string1);

                }
            }
        }
  • [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/q/5431941/2410359). `printf("%s\n", string1);` is UB should `fscanf(fp1, "%[^\n]", string1);` failed to read anything when the line starts with `'\n'`. – chux - Reinstate Monica Jan 27 '19 at 03:47
  • "What the %[^\n] pattern means is "read everything up to the first '\n'" is missing a critical part: `string1` is not changed if 1) the lines starts with `'\n'`, or 2) end-of-file. On rare input error, `string1` is indeterminate. `feof(fp1)` is the wrong substitute for checking the return value of `fscanf()`. – chux - Reinstate Monica Jan 27 '19 at 03:49
  • I'm aware about "while(!feof(file))" not being the ideal way of going about file reading here, but OP needs to understand why his code wasn't working in the first place before rewriting everything following the advised constructs since this is how he will truly understand why the standards are they way they are as opposed to just replicating them. I altered his code as little as possible in order to make it work on my machine and posted the snippet here hoping he would eventually figure out the problem with the rest of his code by himself, that is the best way of learning in my opinion. –  Jan 27 '19 at 12:04
  • 1
    What you posted was a step in the right direction and your [intentions](https://stackoverflow.com/questions/54384348/my-program-in-c-keeps-reading-only-the-first-line-im-trying-to-get-it-to-read/54384991?noredirect=1#comment95587894_54384813) are good. The weakness is that it was not a sufficient step and did not explain remaining vulnerabilities. It still retains UB due to overrun possibility and impact from not checking the `fscanf()` return value. – chux - Reinstate Monica Jan 27 '19 at 14:35
1

.. keeps reading only the first line.

fscanf(fp1, "%[^\n]s", string1); does not read a '\n'.

Since code never read'\n', it remains in the file until some code reads it. Subsequent reading of the file does not occur.


fscanf(fp1, "%[^\n]s", string1); is bad for other reasons too.

1) No width limit. Excessive input will cause undefined behavior (UB). Often that is corruption of other data.

2) The "s" serves no purpose. Drop it.

3) Return value of fscanf() not checked. Code does not know is anything was written in to string1.


Alternative: Use fgets().

        // Avoid
        // while(! feof(fp1)) { 
        //    fscanf(fp1, "%[^\n]s", string1); 
        //    while (! feof(fp2)) {
        //        fscanf(fp2, "%[^\n]s", string2); 

        while(fgets(string1, sizeof string1, fp1)) {
            string1[strcspn(string1, "\n")] = '\0';  // Lop off potential \n
            while(fgets(string2, sizeof string2, fp2)) {
                string2[strcspn(string2, "\n")] = '\0';
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • Thank you, this helped tremendously. I hope i can continue to use this method and understand further as to why my previous while (! feof) method was incorrect. – br34k Jan 27 '19 at 07:07
  • @br34k `! feof(fp1)` informs that an end-of-file occurred _in the past_. When `feof()` is false, it does not foretell the following `fscanf(fp1, "%[^\n]s", ...` success. – chux - Reinstate Monica Jan 27 '19 at 14:27