6
\$\begingroup\$

I need to encrypt and decrypt text, image and video with a program in C with a matrix.

My program is working but it's really slow, how can I improve its speed ? specially for the decrypt part.

Each matrix have an identity part and a filling part.

For encryption, I only do a matrix multiplication of each hex of ascii I get from files.

There is my code for decryption:

In source_decode I have the name of the file I need to decrypt.

In matrice_identity I have an array that contain the order of the matrix identity hidden in the matrix. For example for a matrix like:

10001111
11000111
10100100
10010010

The matrice_identity will contain {4,1,2,3}, because the column 4 (where columns are numbered starting from 0) is 1000, the column 1 is 0100, column 2 is 0010, and column 3 is 0001. Other columns are just here to fill.

void decode_it(char* source_decode ,int* matrice_identity){

    FILE* source_file = NULL;
    source_file = fopen(source_decode, "rb");

    // that's to make difference between the original one, and the decrypted one.

    char* exetention = "d";
    strcat(source_decode, exetention);

    FILE* decoded_file = NULL;
    decoded_file = fopen(source_decode, "wb");

    if(decoded_file != NULL){
    int carac;

        if(source_file != NULL){
            int i = 0;
            int j = 0;
            int octet1[8] = {0};
            int octet2[8] = {0};
            int result[8] = {0};

            //encrypted file are writed in acsii, so here, i get an ascii number.
            carac = fgetc(source_file);

            while(feof(source_file) == 0){

                 //this function convert an ascii number to a byte.
                 convert_hex_bin_v2(carac, octet1);
                 carac = fgetc(source_file);
                 convert_hex_bin_v2(carac, octet2);

                 //This reason of why i'm getting 2 carac is beacause that I need to decrypt 2 bytes to have the original ascii.

                //For example a letter 'a' encrypted give me 2 bytes.

            for(j=0; j<4; j++){
                for(i=0; i<8; i++){
                    if(i == matrice_identity[j]){
                        result[j] = octet1[i];
                        result[j+4] = octet2[i];
                    }
                }
            }


            int k = 0;
            int printFile = 0;
            for(i=7; i>=0; i--){
                printFile += result[i] * pow(2.,(double)k);
                k++;
            }
            fprintf(decoded_file, "%c", printFile);

            carac = fgetc(source_file);
        }
        fclose(decoded_file);

        }else{
        printf("read failed");

        }

        fclose(source_file);

    }else{
        printf("output creation failed");
    }
}

And there is the function that convert hex to binary:

void convert_hex_bin_v2(int carac, int* octet){

        int x;
          for (x = 0; x < 8; x++) {
            octet[x] = 0;
          }

    if(carac >= 128){

     octet[0]=1;
     carac -=128;
    }
    if(carac>=64){

     octet[1]=1;
     carac -=64;
    }
    if(carac>=32){
     octet[2]=1;
     carac -=32;
    }
    if(carac>=16){
     octet[3]=1;
     carac -=16;
    }
    if(carac>=8){
     octet[4]=1;
     carac -=8;
    }
    if(carac>=4){
     octet[5]=1;
     carac -=4;
    }
    if(carac>=2){
     octet[6]=1;
     carac -=2;
    }
    if(carac>=1){
     octet[7]=1;
     carac -=1;
    }
}
\$\endgroup\$
1
\$\begingroup\$

Welcome to Code Review, nice first question!

Perform Error Checking as Soon as Possible
The complexity of the code can be decreased by performing earlier error checking in the function decode_it. If the code was changed so that source_file was tested at the beginning there would be less indentation and the code would be easier to follow:

void decode_it(char* source_decode ,int* matrice_identity){

    FILE* source_file = NULL;
    source_file = fopen(source_decode, "rb");
    if (source_file == NULL)
    {
        printf("read failed");
        return;
    }

Don't Assume the Buffer has the Necesary Capacity
There is a possible critical error in the following code:

    char* exetention = "d";
    strcat(source_decode, exetention);

The variable source_code may not have enough space allocated to append the extention. This can cause the program to crash, especially when the code is optimized. Allocate a new string with enough space to accept the file name and the extension.

    char* decoded_fname = malloc(strlen(source_decode) + strlen(exetention) + 1);  // leave room for NULL termination
    strcpy(decoded_fname, source_decode);
    strcat(source_decode, exetention);

Reduce Complexity, Follow SRP

The Single Responsibility Principle states that every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.

Robert C. Martin expresses the principle as follows:

A class should have only one reason to change.

While this is primarily targeted at classes in object oriented languages it applies to functions and subroutines in procedural languages like C as well.

The decode_it() function can be broken up into smaller functions to make it more readable, and to allow you to profile the program to see where you are spending the most time.

The following is a good candidate for a function:

            for (j = 0; j<4; j++) {
                for (i = 0; i<8; i++) {
                    if (i == matrice_identity[j]) {
                        result[j] = octet1[i];
                        result[j + 4] = octet2[i];
                    }
                }
            }

Another good candidate for a function is the code

            int k = 0;
            int printFile = 0;
            for (i = 7; i >= 0; i--) {
                printFile += result[i] * (int)pow(2., (double)k);
                k++;
            }

Please note in the previous I cast the result of pow function to int, my compiler gave me a warning message on that line.

Possible Optimizations
Rather than reading the input file character by character input large chunks of data (file block size perferably) into a large buffer internal to the program and process the buffer instead. Each time fgetc is called it is making a system call which is expensive(time consuming). Having an input buffer and processing it may help reduce the complexity of the code as well.

For the same reason, create a string output buffer and write it to the file all at once rather than repeatedly calling fprintf().

Using pointers rather than array indexes may also improve performance.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for this answer. Will try all the optimization you've suggest. Come back asap with news. \$\endgroup\$ – Jilu Dec 9 '17 at 18:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.