fscanf stucks on the first line alone

Question

I'm doing an entry level Lexical analyser here. My code is

 bool DfaTab :: isAccepted(string s){

        FILE *fp1;
        int i;

        fp1 = fopen("javalist.dat","r");

        while(!(feof(fp1))){

            fscanf(fp1,"%s%d%[^\n]",tkname.c_str(),&tkval);

            if(strcmp(s.c_str(),tkname.c_str()) == 0){

                setTkVal(tkval);
                setTkName(tkname.c_str());
                state = true;
                return state;
                break;
            }
            else{
                //What should I do here to skip to next line
            }
        }

        return state;

        fclose(fp1);
    }

which will be called from here :

while (!(feof(src))) {

        fscanf(src,"%s[^\t^ ^\n]",sym.c_str());

        if (d.isAccepted(sym)) {

            fprintf(des,"<%s, %d>\n",d.getTkName().c_str(),d.getTkVal());

        }

        else{

            cout << "Can not find symbol " << d.getTkName().c_str();
            cout << "Rejected.\n";
            break;

        }
    }

My problem is that the fscanf() function which is in the isAccepted() function does not skip to the new line and repeatedly printing the the first line that was read at the beginning of the is printed rest of the execution. What should I do now?

the file contains :

//javalist.dat
    boolean     0
    brake       1
    case        2
    catch       3
    const       4
    continue    5
    default     6
    ....

Why are you using `fscanf()` instead of `std::ifstream` 1st place? Also [read here](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) please, though this refers to c++ specifically. — πάντα ῥεῖ, Oct 19 '14 at 13:01
Would be great if you could supply a simple text file that gives you the same problem. — Nard, Oct 19 '14 at 13:11
Also, `c_str()` returns a pointer to a **read-only** `char` so that might cause some problems. You can use the return value of fscanf to see how many of your tokens were successfully extracted from the file. — Nard, Oct 19 '14 at 13:15
Also always check the return value of any `scanf` family function call. If parsing fails and you proceed ignoring that, your program rarely does what you want, and might do something you do *not* want, depending on circumstances... — hyde, Oct 19 '14 at 13:33
Searching through a file each time you want to see if something's a keyword will get prohibitively slow very quickly. — molbdnilo, Oct 19 '14 at 14:53

Evan Teran · Accepted Answer · 2014-10-19T15:56:32.967

It is incorrect to call feof before you have performed a read operation on the file. You should probably restructure your code to be like this:

// some reasonble max size
char buf[1024];

// read in a line
while(fgets(buf, sizeof(buf), fp1)) {

    // parse the contents of that line
    sscanf(buf, ...); //

    // the rest of your code...
}

Additionally, you have a fairly major bug in your code.

fscanf(fp1,"%s%d%[^\n]",tkname.c_str(),&tkval);

The result of tkname.c_str() is NOT a writable string, it's a const char *. You may not pass it to fscanf to be written to, that is well into undefined behavior and can easily lead to crashes. Instead you need to allocate a new buffer, read into that and then assign it to the string.

Alternatively, you can use iostreams to solve this much simpler:

bool DfaTab :: isAccepted(string s){
    std::ifstream file("javalist.dat");

    std::string line;
    std::string name;
    int val;

    // read a line
    while(std::getline(file, line)) {

        // get the data out of the line
        std::stringstream ss(line);
        if(ss >> name >> val) {
            if(name == s) {
                setTkVal(val);
                setTkNamename.c_str()); // if this method takes a std::string, no need for the .c_str()
                state = true;
                return state;
            } else{
                // just ignore this line and it'll continue
            }
        } else {
            // an invalid line, handle as you please...
        }
    }    
    return state;
}

Notice how this solution is actually simpler overall.

fscanf stucks on the first line alone

1 Answers1