How to free memory properly?-CodePudding

Please help, I can't figure out how to free memory correctly. There is a code below

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

#define INT_MSG_LEN 25;

enum {NO_ERROR = 0, 
      ERROR_INPUT = 100, 
      ERROR_LEN = 101};

static const char *error_texts[] = { "Error input!", 
                                     "Error lenghts!"};

void shift(char *msgEnc, char *msg, char *msgRes, char *mainMsg, char *alphabet, int offset);
void report_error(int error);
void print_error(int error);
int get_sameletters(char *msg, char *msgRes, int offset);
int get_letter(char letter, char *alphabet);
int compare(char *msgEnc, char *msg, char *msgRes, char *alphabet, int offset);
char *read_Input_Msg(int *msglen);
char rotate(char *original, int offset);

int main(int argc, char *argv[])
{
    int ret = NO_ERROR;
    char *msgEnc, *msg, alphabet[53] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
    int msgEncLen, msgLen;

    msgEnc = msg = NULL;
    msgEncLen = msgLen = 0;

    msgEnc = read_Input_Msg(&msgEncLen);
    if (msgEnc)
        msg = read_Input_Msg(&msgLen);

    if (msgEnc == NULL || msg == NULL)
        ret = ERROR_INPUT;
    else if (msgEncLen != msgLen)
        ret = ERROR_LEN;
    
    char msgRes[msgEncLen], mainMsg[msgEncLen];
    
    if (ret == NO_ERROR)
        shift(msgEnc, msg, msgRes, mainMsg, alphabet, msgEncLen);
    else
        print_error(ret);

    free(msgEnc);
    free(msg);
    return ret;
}

void shift(char *msgEnc, char *msg, char *msgRes, char *mainMsg, char *alphabet, int offset)
{//function for decoding text by a defined offset
    int dis;
    dis = compare(msgEnc, msg, msgRes, alphabet, offset);
    for (int i = 0; i<offset;   i)
            mainMsg[i] = msgEnc[i] dis;

    rotate(mainMsg, offset);
    for(int i = 0; i<offset;   i)
        printf("%c", mainMsg[i]);
    printf("\n");
}

void report_error(int error)
{//prints error 
    if (error >= ERROR_INPUT && error <= ERROR_LEN)
        fprintf(stderr, "%s\n", error_texts[error - ERROR_INPUT]);
}

void print_error(int error)
{//what error it is
    switch (error){
        case ERROR_INPUT:
            report_error(ERROR_INPUT);
            break;
        case ERROR_LEN:
            report_error(ERROR_LEN);
            break;
    }
}

int get_sameletters(char *msg, char *msgRes, int offset)
{//gets count of sameletters between two strings
    int sameLetters = 0;
    for (int i = 0; i<offset-1;   i){
        if (msg[i] == msgRes[i])
            sameLetters  ;
    }
    return sameLetters;
}

int get_letter(char letter, char *alphabet)
{   
    int k = 0;
    for (int i=0; alphabet[i];   i){
        if (letter == alphabet[i])
            k = i;
    }
    return k;
}

int compare(char *msgEnc, char *msg, char *msgRes, char *alphabet, int offset)
{//calculate a distance between first input string and string what will get after decryption
    int distance, max_letters = 0;
    for (int i = 0; alphabet[i];   i){
        for (int j = 0; msgEnc[j];   j){
                msgRes[j] = alphabet[(get_letter(msgEnc[j], alphabet)   i) % 52];
            }
            int sameLetters = get_sameletters(msg, msgRes, offset);
            if (sameLetters >= max_letters){
                max_letters = sameLetters;
                distance = i;
            }
    }
    return distance;
}

char *read_Input_Msg(int *msglen)
{//input messages, at the same time counts the length of the entered string
    int capacity = INT_MSG_LEN;
    char *msg = malloc(capacity);
    int c, len = 0;
    while ((c = getchar()) != EOF && c != '\n'){
        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'))) {
            free(msg);
            msg = NULL;
            len = 0;
            break;
        }
        if (len == capacity){
            char *tmp = realloc(msg, capacity * 2);
            if (tmp == NULL){
                free(msg);
                msg = NULL;
                len = 0;
                break;
            }
            capacity *= 2;
            msg = tmp;
        }
        msg[len  ] = c;
    }
    *msglen = len;
    return msg;
}

char rotate(char *original, int offset)
{
    for (int i = 0; i<offset;   i){
        if (original[i] > 'Z' && original[i]<'a')
            original[i]  = 6;
        else if (original[i] > 'z'){
            int k = (int)original[i];
            k -= 58;
            original[i] = (char)k;
        }
    }
    return *original;
}

When I run it through Valgrind, it says to me that I have errors with the allocated memory, says I do not free it. Writes that memory is not freed at 107.56 and 44 lines, more precisely

==56665== Conditional jump or move depends on uninitialised value(s)
==56665==    at 0x4015BF: compare (main.c:107)
==56665==    by 0x401468: shift (main.c:56)
==56665==    by 0x4012C3: main (main.c:44)
==56665== 
�elloword
==56665== 
==56665== HEAP SUMMARY:
==56665==     in use at exit: 0 bytes in 0 blocks
==56665==   total heap usage: 4 allocs, 4 frees, 2,098 bytes allocated
==56665== 
==56665== All heap blocks were freed -- no leaks are possible
==56665== 
==56665== Use --track-origins=yes to see where uninitialised values come from
==56665== For lists of detected and suppressed errors, rerun with: -s
==56665== ERROR SUMMARY: 52 errors from 1 contexts (suppressed: 0 from 0)

It seems to have freed the memory, but this is not enough, because there is unfreed memory after some function calls. I guess I just don't know how to free it. At the end of each function that asks for additional memory, I tried to free it through free (the array that the function called), but in this case it displays a segmentation fault. I would be grateful if you let me know at least something.

here's example to input

xUbbemehbT
XYlloworld

CodePudding user response：

The valgrind output does not say that you have a memory leak or that you didn't free something properly.

If you look at this part of the output:

==56665== HEAP SUMMARY:
==56665==     in use at exit: 0 bytes in 0 blocks
==56665==   total heap usage: 4 allocs, 4 frees, 2,098 bytes allocated
==56665== 
==56665== All heap blocks were freed -- no leaks are possible

It states that all memory blocks that were allocated were freed. So you don't have an allocation problem.

The important part is this part:

==56665== Conditional jump or move depends on uninitialised value(s)
==56665==    at 0x4015BF: compare (main.c:107)
==56665==    by 0x401468: shift (main.c:56)
==56665==    by 0x4012C3: main (main.c:44)

This says that at the compare function at line 107 (which was called from shift at line 56, which in turn was called from main at line 44), a value that was never initialized was read. This line is:

for (int j = 0; msgEnc[j];   j){

So this tells us you're reading a byte in the msgEnc array that was never written to. This array was written to in the read_Input_Msg function which reads one character at a time into the array.

Since this is array is supposed to be a string, and because you're reading a byte at a time, you need to manually add a terminating null byte which you're not doing. So when compare is eventually called, the above line is looking for a null byte which was never explicitly written, so you end up reading uninitialized bytes.

After the main while loop in read_Input_Msg, add the null byte at the end of the string:

while ((c = getchar()) != EOF && c != '\n'){
    ...
}
msg[len]=0;