I am having trouble debugging my implementation of Vigenere's cipher in C. The error arises when using file input (-f flag) where the file contains fewer than 6 chars (+ 1 EOF), it spits out some number of random characters as well as the expected input and I cannot figure out why this is, although I suspect it has something to do with the second part of my question which is, when using fread(), I noticed that this
if( fread(fcontents, fsize, sizeof(char), file) != 1 ) {...}
will run with no issues, whereas this
if( fread(fcontents, sizeof(char), fsize, file) != 1 ) {...}
doesn't work (i.e. causes fread() to return 1 and trigger the error handling code beneath it), which I would expect to be the other way around according to answers from here, but I may just be misinterpreting something.
My complete code is as follows:
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#include <unistd.h>
#define ENC 0 //Encrypt mode
#define DEC 1 //Decrypt mode
#define INP 0 //Commandline input mode
#define FLE 1 //File input mode
typedef struct {
char *password;
char *file_name;
char *input;
int edmode;
int ifmode;
} options;
void string_clean(char *source)
{
char *i = source;
char *j = source;
while(*j != 0) {
*i = *j++;
if( *i != ' ' && (isupper(*i) || islower(*i)) )
i++;
}
*i = 0;
}
char *ftostr(char *file_name) //Takes a file name as input and returns a char* to the files contents, returns NULL pointer on faliure. Allocated string must be freed after use by parent function to prevent memory leaks.
{
FILE *file;
long int fsize;
char *fcontents;
if( !(file = fopen(file_name, "r")) ) {
fprintf(stderr, "Error opening file \"%s\"!\n", file_name);
return NULL;
}
fseek(file, 0L, SEEK_END);
fsize = ftell(file);
rewind(file);
if( !(fcontents = malloc((fsize + 1) * sizeof(char))) ) {
fclose(file);
fprintf(stderr, "Error allocating memory!");
return NULL;
}
if( fread(fcontents, fsize, sizeof(char), file) != 1 ) { //suspected buggy line
fclose(file);
free(fcontents);
fprintf(stderr, "Error copying file to memory!\n");
return NULL;
}
fclose(file);
return fcontents;
}
options parse_opts(int argc, char *argv[])
{
int c;
options args;
args.edmode = ENC; //enable encrypt mode by default
args.ifmode = INP; //enable commandline input mode by default
args.file_name = NULL;
args.password = NULL;
args.input = NULL;
opterr = 0;
while((c = getopt(argc, argv, "dep:i:f:")) != -1) {
switch(c) {
case 'e':
args.edmode = ENC;
break;
case 'd':
args.edmode = DEC;
break;
case 'p':
args.password = optarg;
break;
case 'i':
args.input = optarg;
args.ifmode = INP;
break;
case 'f':
args.file_name = optarg;
args.ifmode = FLE;
break;
case '?':
if(optopt == 'f' || optopt == 'p' || optopt == 'i')
fprintf(stderr, "Option -%c requires an argument.\n", optopt);
else if(isprint(optopt))
fprintf(stderr, "Unknown option `-%c'.\n", optopt);
else
fprintf(stderr, "Unknown option character `\\x%x'.\n", optopt);
fprintf(stderr, "Usage: %s (-f file_name || -i input) -p password [options]\n"
"Optional: -e -d\n", argv[0]);
exit(-1);
}
}
return args;
}
char *vigenere_dec(char cipher_text[], char cipher[])
{
char *plain_text;
string_clean(cipher_text);
string_clean(cipher);
int plain_text_len = strlen(cipher_text);
int cipher_len = strlen(cipher);
if( !(plain_text = malloc((plain_text_len + 1) * sizeof(char))) )
return 0;
for(int i = 0; i < cipher_len; i++) {
if(isupper(cipher[i]))
cipher[i] -= 'A';
else if(islower(cipher[i]))
cipher[i] -= 'a';
}
for(int i = 0, j = 0; i < plain_text_len; i++, j++) {
if(j == cipher_len)
j = 0;
if(isupper(cipher_text[i]))
cipher_text[i] -= 'A';
else if(islower(cipher_text[i]))
cipher_text[i] -= 'a';
plain_text[i] = ((cipher_text[i] - cipher[j]) % 26);
if(plain_text[i] < 0)
plain_text[i] += 26;
plain_text[i] += 'A';
}
return plain_text;
}
char *vigenere_enc(char plain[], char cipher[])
{
char *cipher_text;
string_clean(plain);
string_clean(cipher);
int plain_len = strlen(plain);
int cipher_len = strlen(cipher);
if(plain_len == 0 || cipher_len == 0)
return NULL;
if( !(cipher_text = malloc((plain_len + 1) * sizeof(char))) )
return NULL;
for(int i = 0; i < cipher_len; i++) {
if(isupper(cipher[i]))
cipher[i] -= 'A';
else if(islower(cipher[i]))
cipher[i] -= 'a';
}
for(int i = 0, j = 0; i < plain_len; i++, j++) {
if(j == cipher_len)
j = 0;
if(isupper(plain[i]))
plain[i] -= 'A';
else if(islower(plain[i]))
plain[i] -= 'a';
cipher_text[i] = ((plain[i] + cipher[j]) % 26) + 'A';
}
return cipher_text;
}
int main(int argc, char *argv[])
{
options args;
char *output_text = NULL;
args = parse_opts(argc, argv);
if(args.password == NULL) {
fprintf(stderr, "Password uninitialised!\n");
exit(-1);
}
if(args.input == NULL && args.file_name == NULL) {
fprintf(stderr, "Input stream uninitialised!\n");
exit(-1);
}
if(args.ifmode == INP) {
if(args.edmode == ENC)
output_text = vigenere_enc(args.input, args.password);
else if(args.edmode == DEC)
output_text = vigenere_dec(args.input, args.password);
} else if(args.ifmode == FLE) {
if( !(args.input = ftostr(args.file_name)) )
return -1;
if(args.edmode == ENC)
output_text = vigenere_enc(args.input, args.password);
else if(args.edmode == DEC)
output_text = vigenere_dec(args.input, args.password);
free(args.input);
}
puts(output_text);
free(output_text);
return 0;
}
The fault is unterminated strings. You allowed room for the termination char with
if( !(plain_text = malloc(plain_text_len + 1)) ) // (simplified)
but after you have set, for example
plain_text[i] += 'A';
you need to end the string with
plain_text[i+1] = '\0';
or when the string is complete.
For the second part, you quoted another question, but failed to see that fread() returns the number of items read. So if you swap its size and count arguments, expect a different result (unless fsize == 1).
So you can either use
if( fread(fcontents, fsize, 1, file) != 1 ) {...}
or this
if( fread(fcontents, 1, fsize, file) != fsize ) {...}
Note I changed sizeof(char) to 1 since by definition, it is.