Why doesn't my simple XOR encryption program translate the characters right, and why does it add more characters at the end?

Question

I'm making a XOR based en/decryptor, that works like this. You have a plaintext character, for example 3, and a user key, for example 5. Written in bits:

3 = 00000011

5 = 00000101

Now if we do XOR operation, we get 6:

6 = 00000110

This can be reversed by saying 6 XOR 5, which is 3.

So I have made this program. But it's really buggy, it doesn't translate the text right, and it adds a lot of characters in the end of the file, depending which key you are using.

using namespace std;
int main(int argc, char *argv[])
{
    char buffer[5001]; 
    ifstream fin("a.txt", ifstream::in);
    ofstream fout("b.txt");
    int key;
    char znak;

    // console    
    cout << "Key: ";
    cin >> key;
    fin.get(znak);
    while(!fin.eof() && znak != ' ')
    {
       fin.get(buffer, sizeof(buffer));     
    }

    for(int i = 0; i < sizeof(buffer); i++)
    {
       fout << function(key, buffer[i]);
    }

    cout << "done" << endl;

    cin.get();
    return 0;
}

char function(int key,char input)
{
    return input ^ key;
}

Why doesn't the program translate the text right? And Why does it add characters to the end of the file?

Answer 1

From the looks of things, the encryption has almost nothing to do with anything (or at least with the problem) here.

while(!fin.eof() && znak != ' ')

A loop of the form while (!whatever.eof()) is pretty much guaranteed to work incorrectly.

    fin.get(buffer, sizeof(buffer)); 

When you do this, note that it does not guarantee that it will read sizeof(buffer) characters -- only that it won't read any more than that. You typically get fewer at the end of the file, when there simply aren't that many characters left to read. [Edit: I should also mention that you can read fewer in other places as well -- e.g., if you're reading from a network connection, it's fairly common to receive a partial buffer, but more later when it arrives.]

for(int i = 0; i < sizeof(buffer); i++)

So here, when you attempt to process sizeof(buffer) characters, chances are pretty good that (especially on the last iteration) you're attempting to process more characters than you actually read. You can retrieve the number you read with gcount, though I'm not sure I'd really recommend using it.

Personally, I'd probably do something like this:

class function { 
    char key;
public:
    function(char k) : key(k) { }
    char operator()(char input) { return key ^ input; }
};

int main() { 
    std::ifstream fin("a.txt");
    std::ofstream fout("b.txt");

    fin.noskipws();

    std::transform(std::istream_iterator<char>(fin),
                   std::istream_iterator<char>(),
                   std::ostream_iterator<char>(fout),
                   function(key));
    return 0;
}