Trouble with reading/writing files

This program is quite simple, it reads in part of a given file and after creating a new entry rewrites the file with the new entry. It works great up until 4/5 entries when it stops writing it correctly.

Code:

---

#include <iostream>
#include <fstream>
#include <string>
#include <new>
using namespace std;

ifstream fin;
ofstream fout;
int f_exists;

struct human{
    string first_name;
    string last_name;
};

human create (void) {
    human person;
    if(f_exists!=1)cin.ignore();
    cout << "What is their first name: ";
    getline(cin,person.first_name);
    cout << "What is their last name: ";
    getline(cin,person.last_name);
    return(person);
}

int verify(human hu){
    char valid;
    while(valid != 'y' && valid != 'Y'){
        cout << "First name: " << hu.first_name << endl;
        cout << "Correct so far? (y/n) ";
        cin >> valid;
        if(valid == 'n' || valid == 'N'){
            return(0);
        }
    }
    do{
        cout << "Last name: " << hu.last_name << endl;
        cout << "Correct so far? (y/n) ";
        cin >> valid;
        if(valid == 'n' || valid == 'N'){
            return(0);
        }
    }while(valid != 'y' && valid != 'Y');
    return(1);
}

void write(human hu){
    fout.write("\n\t<person>\n",11);
    fout.write("\t\t<name>\n",9);
    fout.write("\t\t\t<first_name>",15);
    fout.write(hu.first_name.c_str(),hu.first_name.length());
    fout.write("</first_name>\n",14);
    fout.write("\t\t\t<last_name>",14);
    fout.write(hu.last_name.c_str(),hu.last_name.length());
    fout.write("</last_name>\n",13);
    fout.write("\t\t</name>\n",10);
    fout.write("\t</person>",10);
}

int main (void){
    string line;
    int nl = 0;
    char f_conf,start;
    char * file;
    int size,correct;
    string header,footer,f_name;
    human new_entry;
    while(start != 'y' && start != 'Y'){
        cout << "If you don't finish this your file will be destroyed.\nI'm serious, the file will go bye-bye so make sure you want to run it first.\nDo you want to start? (y/n) ";
        cin >> start;
        if(start == 'n' || start == 'N')return 0;
    }
    cin.ignore();
    header = "<?xml version=\"1.0\" encoding=\"UFT-8\"?>\n<!DOCTYPE contacts SYSTEM \"http://www.arenlor.com/contacts/contacts.dtd\">\n<contacts>\n";
    footer = "\n</contacts>";
    while(f_conf != 'y' && f_conf != 'Y' && f_conf != 'n' && f_conf != 'N'){
        cout << "Do you already have a file? (y/n)";
        cin >> f_conf;
        if(f_conf == 'n' || f_conf == 'N') f_name = "contacts.xml";
        else if(f_conf == 'y' || f_conf == 'Y'){
            cin.ignore();
            cout << "What is its name: ";
            getline(cin,f_name);
            f_exists = 1;
        }
    }
    fin.open(f_name.c_str());
    fin.seekg(0,ios::end);
    size = fin.tellg();
    if(size>141)size -= 141;
    fin.seekg(0,ios::beg);
    while(getline(fin,line)){
        nl++;
    }
    fin.clear();
    nl -= 6;
    size -= nl;
    file = new char[size];
    fin.seekg(127,ios::beg);
    fin.readsome(file,size);
    fin.close();
    fout.open(f_name.c_str());
    fout.write(header.c_str(),124);
    fout.write(file,size);
    delete[] file;
    do{
        cin.clear();
        new_entry = create();
        correct = verify(new_entry);
    }while(correct == 0);
    write(new_entry);
    fout.write(footer.c_str(),12);
    fout.close();
    return 0;
}

---

An example of how it stops working:

Code:

---

<?xml version="1.0" encoding="UFT-8"?>
<!DOCTYPE contacts SYSTEM "http://www.arenlor.com/contacts/contacts.dtd">
<contacts>
        <person>
                <name>
                        <first_name>Arenlor</first_name>
                        <last_name>BloodLeaf</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Dove</first_name>
                        <last_name>Ashby</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Jordan</first_name>
                        <last_name>Morris</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Nanci</first_name>
                        <last_name>Lycett</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Faith</first_name>
                        <last_name>Kelly</last_name>
                </name>
        </person>
</contacts>

---

Code:

---

<?xml version="1.0" encoding="UFT-8"?>
<!DOCTYPE contacts SYSTEM "http://www.arenlor.com/contacts/contacts.dtd">
<contacts>
        <person>
                <name>
                        <first_name>Arenlor</first_name>
                        <last_name>BloodLeaf</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Dove</first_name>
                        <last_name>Ashby</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Jordan</first_name>
                        <last_name>Morris</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Nanci</first_name>
                        <last_name>Lycett</last_name>
                </name>
        </person>
        <person>
                <name>
                        <first_name>Faith</first_name>
                        <last_name>Kelly</last

                        <last_name>Kelly</last
        <person>
                <name>
                        <first_name>Jerod</first_name>
                        <last_name>Lycett</last_name>
                </name>
        </person>
</contacts>

---

This is how it always breaks where it stops at the _ in last_name, creates two newlines, repeats the broken last_name line and then correctly adds the new entry. I've tried stepping through Dev-C++'s debugger with no results of where it's going wrong.

Replace this:

Code:

---

void write(human hu){
    fout.write("\n\t<person>\n",11);
    fout.write("\t\t<name>\n",9);
    fout.write("\t\t\t<first_name>",15);
    fout.write(hu.first_name.c_str(),hu.first_name.length());
    fout.write("</first_name>\n",14);
    fout.write("\t\t\t<last_name>",14);
    fout.write(hu.last_name.c_str(),hu.last_name.length());
    fout.write("</last_name>\n",13);
    fout.write("\t\t</name>\n",10);
    fout.write("\t</person>",10);
}

---

with this

Code:

---

void write(human hu){
    fout << "\n\t<person>\n";
    fout << "\t\t<name>\n";
    fout << "\t\t\t<first_name>";
    fout << hu.first_name;
    fout << "</first_name>\n";
    fout << "\t\t\t<last_name>";
    fout << hu.last_name;
    fout << "</last_name>\n";
    fout << "\t\t</name>\n";
    fout << "\t</person>";
}

---

No still same result, I'm willing to redo this if needed.

Quote:

---

Originally Posted by Arenlor

This program is quite simple,

---

This code is suspicious:

Code:

---

size = fin.tellg();
    if(size>141)size -= 141;
    fin.seekg(0,ios::beg);
    while(getline(fin,line)){
        nl++;
    }
    fin.clear();
    nl -= 6;
    size -= nl;
    file = new char[size];

---

Your doing a lot of "on the edge" work here, where if you're off by a single byte anywhere, you are risking a memory overwrite.

Also, why the magic numbers of 141, 124, etc. in your program? Can you guarantee that these numbers are correct? By guarantee, I mean in code and not by "intuition", can you guarantee that those are correct? What guarantees do you have that "size" is big enough for use later in the code?

Here is an example of this in your code:

Code:

---

fout.write(header.c_str(),124);

---

Why 124? What if header contains text that is less than or more than 124 bytes? Shouldn't it be something like this?

Code:

---

fout.write(header.c_str(), header.size());

---

Another thing -- you don't need "new" here:

Code:

---

code *file;
//...
file = new char [size];
//...
delete [] file;

---

This accomplishes the same thing, and is much safer:

Code:

---

#include <vector>
//...
std::vector<char> file( size );
//...
fin.readsome(&file[0],size);
//...
fout.write(&file[0],size);

---

There is no need for delete [], and if your code will contain multiple exit points, there is no chance of you forgetting to call delete[].

Regards,

Paul McKenzie

header is statically set in the code. It's exactly 124 bytes. footer is 12 bytes (also set statically). Together with /r/n for Windows the file would be 141 bytes without any actual information being in it. I'm in the middle of debugging and idiot proofing that whole section, but for now that make-shift hack works to prevent errors when it's not a full file, or when it's a new file.

Code:

---

    while(getline(fin,line)){
        nl++;
    }
    fin.clear();
    nl -= 6;
    size -= nl;

---

I was having trouble with newlines due to /r/n and this fixes it, it just counts the number of lines in the file, ignores the six that are there from the header and footer, and then subtracts that number from size. Otherwise it reads in some of the footer.

New[] and delete[] haven't been causing any trouble but I'll try using vectors anyway. Pretty much half of the code is patches for problems I was having.

Quote:

---

Originally Posted by Arenlor

I was having trouble with newlines due to /r/n and this fixes it, it just counts the number of lines in the file, ignores the six that are there from the header and footer, and then subtracts that number from size. Otherwise it reads in some of the footer.

---

All of these manipulations should indicate that you should have opened the file in binary mode, not text mode.

Code:

---

fin.open(f_name.c_str(), std::ios::binary);

---

Opening a file in text mode as you've done, and then try and pin down the exact byte where you are in the file is haphazard. You are now literally fighting with the way that the runtime reads and saves a text file (i.e. the carriage return / line feed combination, and the EOF character being automatically done by the runtime).

This type of file manipulation that you're doing requires you open it in binary, where there is no CR/LF translation going on, and you have control over which byte in the file you are reading / writing.

Regards,

Paul McKenzie