Could someone tell me why copying a file like this doesn't work?

Question

I am trying to copy an exe file byte by byte. I compared the hex files of the 2 and they are quite different. It seems like some values are not getting loaded in..

#include <iostream>
#include <fstream>
#include <vector>

using namespace std;

int main(){

    ifstream fin("file.exe", ifstream::binary);
    vector<char> buffer(1, 0);


    ofstream newfile;
    newfile.open("newfile.exe", ios::binary);

    while (fin.read(buffer.data(), buffer.size())){
        streamsize s = fin.gcount();
        for (int i = 0; i < buffer.size(); i++){
            if (buffer[i] != EOF){
                newfile << buffer[i];
                cout << buffer[i] << endl;
            } else {
                break;
            }

        }
    }
}

Do you need the streamsize s = fin.gcount() statement? You don't use its result, anyway. — Philipp Claßen, May 28 '15 at 19:23
The vector is in case I want to start grabbing more than one byte at a time. I compare to EOF because that has worked so far. — Jlegend, May 28 '15 at 19:27
The most obvious problem is `buffer[i] != EOF`. Since `buffer[i]` contains a character and `EOF` is not a character, this comparison makes no sense. — David Schwartz, May 28 '15 at 19:28
@Jlegend: Yet here you are complaining that it _doesn't_ work.... — Lightness Races in Orbit, May 28 '15 at 19:29
One issue is that the file stream is a *block* device, meaning it likes to transfer data in large blocks. I suggest a minimum size of the vector to be 512 (a common sector size). — Thomas Matthews, May 28 '15 at 20:54

score 7 · Answer 1 · edited May 23 '17 at 11:51

Why do you read into and write out of a vector with a single char? What is the purpose of s? Why do you attempt to compare against EOF with IOStreams? This code appears to be a bizarre mixture of C and C++, with the result being entirely broken!

Here's how I'd do it:

#include <iostream>
#include <fstream>
#include <iterator>
#include <algorithm>

int main()
{
    std::ifstream fin ("file.exe",    std::ios::binary);
    std::ofstream fout("newfile.exe", std::ios::binary);

    std::copy(
       std::istream_iterator<char>(fin),
       std::istream_iterator<char>(),
       std::ostream_iterator<char>(fout)
    );
}

Or even just:

#include <iostream>
#include <fstream>

int main()
{
    std::ifstream fin ("file.exe",    std::ios::binary);
    std::ofstream fout("newfile.exe", std::ios::binary);

    fout << fin.rdbuf();
}

No muss, no fuss!

This works really well for streams in the general case, but, if all you want to do is perform a byte-for-byte file copy, you'd get your OS to do it for you. It can do it much faster than you can! For example, CopyFile on Windows.

I recommend the OP find an OS API for this. The OS should have some optimized file copying functions already tested. — Thomas Matthews, May 28 '15 at 19:26

score 0 · Answer 2 · answered May 28 '15 at 19:59

thank you for all the help. This is how I ended up getting the job done.

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

int main() {
    ifstream is("file.exe", ios::binary);
    ofstream fs("newfile.exe", ios::binary);
    if (is) {
        is.seekg(0, is.end);
        int length = is.tellg();
        is.seekg(0, is.beg);

        char * buffer = new char[length];

        is.read(buffer, length);
        is.close();

        for (int i = 0; i < length; i++) {
            fs << buffer[i];
        }

        delete[] buffer;
    }
}

Could someone tell me why copying a file like this doesn't work?

2 Answers2