toBase64 encoder

Question

My code convert text to Base64. I used the algorithm without bit operations. What do you think about my code?

#include <stdio.h>
#include <stdlib.h>

#define CRT_SECURE_NO_WARNINGS
#define BIN 2
#define MEMORYSIZE 10

void FromSixBitNumbToDec(char *number1, FILE *result){
    char table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
    int i = 0;
    char out = 0;
    int temp_numb = 0;
    int resul = 0;
    int number = atoi(number1);
    int deg = 1; 
    while (i < 6) { 
        temp_numb = number % 10;
        number /= 10;
        resul += (temp_numb * deg);
        deg *= BIN;
        ++i;
    }
    out = table[resul];
    fprintf(result, "%c", out);
}

void FromDecToBin(int number, FILE *output){
    int *numb = (int*)calloc(MEMORYSIZE, sizeof(int)); // Allocates memory for numbers with a minimum size
    int memory = MEMORYSIZE;
    int i = 0;
    int k = 0;
    while (number >= 2) { // Divide the number 10 number system on a finite number of the number system and the remnants of the division in the array. If memory is low, it increased by 2 times
        numb[i] = number % BIN;
        number = number / BIN;
        ++i; // Array size (numb2 array)
        if (i == memory){
            numb = (int*)realloc(numb, (BIN * memory) * sizeof(int)); 
            memory *= BIN;
        }
    }
    numb[i] = number; // The last remnant
    k = i; 
    while (k != 7){ // Supplement to the number of 8-bit
        k++;
        numb[k] = 0;
    }
    int *numb2 = (int*)calloc(k + 1, sizeof(int));  // Allocates new array for number
    memcpy(numb2, numb, (k + 1) * sizeof(int)); // Copy all digits with numb array
    free(numb); 
    for (i = 0; i <= (k / 2); i++) {  // Overturn number
        memory = numb2[i]; 
        numb2[i] = numb2[k - i];
        numb2[k - i] = memory;
    }
    for (i = 0; i <= k; i++) {
        fprintf(output, "%d", numb2[i]);
    }

}

void toBase64(char *argv[]){
    FILE *text = fopen("C:\\b.txt", "r");
    if (NULL == text) {
        printf("Error!");
        fclose(text);
        return;
    } else {
        printf("File was opened\n");
        FILE *output = fopen("C:\\output.txt", "w"); 
        /*fpos_t position;
        fgetpos(output, &position);*/
        char c = 0;
        while (fscanf(text, "%c", &c)!= EOF) { // Convert symbol to Binary
            FromDecToBin(c,output);
        }
        fclose(text);
        fclose(output);
        //fsetpos(output, &position); // Don;t work because I used fclose(output) and open output2. Please Understand how I gotta keep it real!
        //printf("pos = %d\n", position);
        FILE *output2 = fopen("C:\\output.txt", "r");
        FILE *result = fopen("C:\\result.txt", "w");
        char *number = (char*)calloc(6, sizeof(char));
        int i = 0;
        char temp = 0;
        while (!feof(output2)) {
            while (i < 6) { // Take successively six bit and convert to Base64
                if (!feof(output2)){
                    fscanf(output2, "%c", &temp);
                    number[i] = temp;
                    printf("%c", number[i]);
                    i++;
                } else {
                    i /= BIN;
                    for (; i > 0; i--){
                        fprintf(result, "%c", '=');
                    }
                    return;
                }
            }
            printf("\n");
            FromSixBitNumbToDec(number, result);
            i = 0;
        }
        fclose(output2);
        fclose(result);
    }
}

int main(int argc, char *argv[]){
    if (1 < argc){
        printf("Argument has been Received\n");
        toBase64(argv);
    } else {
        printf("Need some arguments!\nExit... \n");
        return 1;
    }
    return 0;
}

Answer 1

1. You should probably take the filenames as arguments instead of hardcoding them.

2. The code looks very nice horizontally, but is vertically crammed. I would suggest including blank lines between variable declaration blocks and things like if/for/while statements.

3. Some variable names are kinda odd to me. Instead of "temp_numb" just do "temp_number". Instead of "resul" do "temp_result". Truncating words is usually a disaster in terms of readability.

Answer 2

I am puzzled by your argument and file handling. Why you need to write to an intermediate file? (Even if you want to avoid bit operations, writing to an intermediate file is silly.) Why bother mentioning argv if you never use it? If given no arguments, I would expect the program to filter standard input to standard output. If there is one argument, I would expect it to treat it as the input filename, and print its output to stdout. If there are two arguments, then I would expect the second one as the output filename.

You don't check for errors when doing fprintf(). There wouldn't be any indication, if, for example, you run out of disk space.

Avoid using calloc() when a fixed-size array will do. Avoid using fprintf() and fscanf() when putc() and getc() will do. More importantly, don't read/write one character at a time. You will get better performance if you read a whole disk block at once into a buffer (about 8 kiB would be a good size, I think).

The work should be decomposed more logically. I suggest the following framework:

#include <assert.h>
#include <stdio.h>
#include <string.h>

/**
 * Converts up to three octets into four encoded characters.
 * Output will be padded if len < 3.
 */
static void base64Encode(const char *in, size_t len, char out[4]) {
    assert(len <= 3);
    // TODO
}

/**
 * Reads from input, performs the encoding, and writes to output.
 * Returns -1 if unable to write to the output.
 */
int toBase64(FILE *input, FILE *output) {
    char inBuf[(8192 / 3) * 3], outBuf[(8192 / 3) * 4];
    int bytesExtra = 0, bytesRead;
    while ((bytesRead = fread(inBuf + bytesExtra, sizeof(char), sizeof(inBuf) - bytesExtra, input))) {
        int validBytes = bytesExtra + bytesRead;
        int i, o;
        for (i = 0, o = 0; i + 3 <= validBytes; i += 3, o += 4) {
            base64Encode(inBuf + i, 3, outBuf + o);
        }
        if (o != fwrite(outBuf, sizeof(char), o, output)) {
            return -1;
        }
        /* Preserve leftover input bytes for the next round */
        bytesExtra = validBytes - i;
        assert(bytesExtra < 3);
        memmove(inBuf, inBuf + i, bytesExtra);
    }
    if (bytesExtra) {
        base64Encode(inBuf, bytesExtra, outBuf);
        if (4 != fwrite(outBuf, sizeof(char), 4, output)) {
            return -1;
        }
    }
    return 0;
}

/**
 * Interprets the arguments, opens the input/output files (if any),
 * calls toBase64(), and displays any I/O errors.
 */
int main(int argc, char *argv[]) {
    FILE *input = stdin, *output = stdout;
    if (argc > 3) {
        fprintf(stderr, "Usage: ...\n");
        return 1;
    }
    if (argc > 1) {
        if (NULL == (input = fopen(argv[1], "r"))) {
            perror("Couldn't open input file");
            return 2;
        }
    }
    if (argc > 2) {
        if (NULL == (output = fopen(argv[2], "w"))) {
            perror("Couldn't open output file");
            return 2;
        }
    }
    if (-1 == toBase64(input, output)) {
        perror("Error writing output");
        return 2;
    }
}