Why is my overloaded >> operator throwing bad_alloc?

Question

I have defined a custom istream operator >> to read from a text file to a container (library) of objects (package)

istream& operator>>(istream& ist, library& lib) //js read packages into library
{
    char c;
    while (ist.get(c)){ //js read in package
        string name;
        while(c != ',' || c!= ';'){ //js read file name
            name += c;  
            ist.get(c);
        }

        vector<string> tagz;
        while(c != ';'){    //js read in tags
            ist.get(c);
            string temptag;
            while(c != ','){    
                temptag += c;
                ist.get(c);
                if(c==',') tagz.push_back(temptag); //js add tag to tagz vector
            }
        package temp(name, tagz); //js define new package
        lib.add(temp);  //js add new package to list
        }
    }
    return ist;
}

However, when I use the operator in my main function, I get the error

terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc

I know this is a memory allocation issue, but I can't figure out where the bug is. Any help or input is appreciated!

Answer 1

I think the issue here might be loops of this form:

 while(c != ';'){    //js read in tags
    ist.get(c);
    ...
 }

In the event that ist encounters an error (for example, reading past the end of the file or if the connection breaks) before a ; is read, then this will go into an infinite loop because ist.get(c) will leave c unchanged. Since your code keeps appending characters to a string, this infinite loop will try to create an unboundedly long string, which will fail when the system runs out of memory for characters.

To fix this, you should probably add some error-checking to this code so that if the stream encounters an error during extraction, your function will exit rather than looping indefinitely.

EDIT: As @uk4321 pointed out, there's also an infinite loop here:

while(c != ',' || c!= ';') {
     ...
}

This statement is always true. Each character has only one value, so every character will compare different to at least one of , and ;. To fix this, try rewriting it as

while(c != ',' && c!= ';') {
     ...
}

Hope this helps!

Answer 2

Look at this code right here:

        string temptag;
        while(c != ','){    
            temptag += c;
            ist.get(c);
            if(c==',') tagz.push_back(temptag); //js add tag to tagz vector
        }

It will create a string, assign it to temptag, and then loop. The temptag variable never gets reassigned, so it will be adding the same one to tagz every time. This means that when tagz gets destructed, it will try to destruct temptag multiple times and throw an exception.