You are performing an action that is meant to operate on binary data, not textual data, therefore you should open your files in binary mode. This is especially important when reading the encoded file.
You are dynamically allocating the CBlowFish object but never deleting it. I would just declare it on the stack for simplicity.
I would also prefer to see better error handling with the fopen() calls. You can handle this quite nicely with a helper function (or a macro).
There is no need to pad your input with null characters. The blowfish code handles this for you. Your responsibility is only to accurately report how much data is in your input buffer (the third parameter to the Encode() method). Even if you did pad your own input, you shouldn't use NULL as a null character. NULL semantically refers to a null pointer. You should use '\0', which refers to a null character.
There is no need to use strlen on a string literal in your code. Declare your string using array notation and use sizeof() to determine the size at compile time (not really a big deal but I thought I should point it out).
I don't believe you need the fseek() calls. After a call to fopen(), the internal pointer should already be at the beginning of the file. I may be wrong about this, but I've never had to fseek() to the beginning of a file on any Windows or Linux system I've worked on.
Lastly, while I was testing this out, I had problems with repeating characters at the end of the decoded file. I realized the decoding process got to the last 2 bytes of the file, it wrote those 2 bytes to the output buffer and left the rest of the buffer the way it was. I had no way of knowing that there were only 2 bytes left (Decode() doesn't tell you how many characters it decoded) so I was writing the whole 8 byte buffer out even though I only needed the first two bytes.
To combat this bug, I had to keep track of the filesize while reading in the file. Maybe there's a better way to handle this, but keeping track of the filesize works.
Anyway, less talking and more code. I think its easier to learn if I just show you some code. Notice I use the C versions of the headers. You can use the C++ headers if you want.
Code:
#include <stdlib.h>
#include <stdio.h>
#include "blowfish.h"
#define my_fopen(fileptr, filename, mode) \
fileptr = fopen(filename, mode); \
if (fileptr == NULL) { \
fprintf(stderr, "Error: Couldn't open %s.\n", filename); \
exit(1); \
}
const char *input_file_name = "test.txt";
const char *encoded_file_name = "encoded.txt";
const char *decoded_file_name = "decoded.txt";
unsigned char key[] = "thisisthekey";
int main(void) {
FILE *infile, *outfile;
int result, filesize;
const int n = 8; // make sure this is a multiple of 8
const int size = 1;
unsigned char input[n], output[n];
CBlowFish bf;
bf.Initialize(key, sizeof(key)-1); // subtract 1 to not count the null terminator
my_fopen(infile, input_file_name, "rb")
my_fopen(outfile, encoded_file_name, "wb")
filesize = 0;
while (result = fread(input, size, n, infile)) {
filesize += result;
fwrite(output, size, bf.Encode(input, output, result), outfile);
}
fclose(outfile);
fclose(infile);
my_fopen(infile, encoded_file_name, "rb")
my_fopen(outfile, decoded_file_name, "wb")
while (result = fread(input, size, n, infile)) {
bf.Decode(input, output, result);
fwrite(output, sizeof(output[0]), filesize < n ? filesize : n, outfile);
filesize -= n;
}
fclose(outfile);
fclose(infile);
return 0;
}
[edit] Actually there's a logic error in my code. Maybe you can spot it? [/edit]
Originally Posted by brewbuck
