0

I wrote some code but am not sure how to properly free memory dynamically allocated inside of a function.

What is the proper way to free memory without causing a heap corruption, or better yet, what is causing the corruption when trying to free `returnedAry``

  #include <stdio.h>
#include <stdlib.h>
#define MAX_NUM_DIGITS 10

void displayMainMenu(void);
int* extractUncommonDigit(int*, int);

int main() {
    displayMainMenu();

    return 0;
}

void displayMainMenu() {
    int* userAry = NULL;
    int* returnedAry = NULL;
    int menuOption;
    int numOfInts;
    int i;

    do {
        printf("\n\n*********************************************\n"
            "*                     MENU                  *\n"
            "* 1. Calling extractUncommonDigit()         *\n"
            "* 2. Quit                                   *\n"
            "*********************************************\n"
            "Select an option (1 or 2): ");
        scanf_s("%d", &menuOption);

        switch (menuOption) {
        case 1:
            printf("\n  How many integers? ");
            scanf_s("%d", &numOfInts);
            userAry = (int*)malloc(numOfInts * sizeof(int));

            for (i = 0; i < numOfInts; i++) {
                printf("    Enter integer #%d: ", i + 1);
                scanf_s("%d", &userAry[i]);
            }

            printf("\n  The original array: \n");
            for (i = 0; i < numOfInts; i++) {
                printf("    %d\n", userAry[i]);
            }

            printf("\n  Calling extractUncommonDigit() -\n");
            returnedAry = extractUncommonDigit(userAry, numOfInts);
            printf("\n  Displaying after returning the array"
                " -- The uncommon digits: ");

            if (returnedAry[0] != 0) {
                printf("\n    There is/are %d uncommon digit(s)", returnedAry[0]);
                for (int i = 1; i < returnedAry[0] + 1; i++) {
                    printf("\n      %d", *(returnedAry + i));
                }
            } else {
                printf("\n   There is/are no uncommon digit(s)");
            }
            break;
        case 2:
            free(userAry);
            /* This line causes heap corruption error
            -----------------------------------------
            free(returnedAry);
            -----------------------------------------
            */
            printf("\n  Fun Excercise ...");
            break;
        default:
            printf("\n  WRONG OPTION!");
            break;
        }
    } while (menuOption != 2);

    free(userAry);

    return;
}

int* extractUncommonDigit(int* ary, int size) {
    int* assembledAry = 0;
    int tmp;
    int** digitInfoAry = (int**)malloc(size * sizeof(int*));
    int i, j;
    int digitAry[10] = { 0 };
    int uncommonCount = 0;

    for (i = 0; i < size; i++) {
        *(digitInfoAry + i) = (int*)calloc(MAX_NUM_DIGITS, sizeof(int));
    }

    for (i = 0; i < size; i++) {
        tmp = *(ary + i) < 0 ? -(*(ary + i)) : *(ary + i);

        do {
            *(*(digitInfoAry + i) + tmp % 10) = 1;
            tmp /= 10;
        } while (tmp != 0);
    }

    for (i = 0; i < MAX_NUM_DIGITS; i++) {
        for (j = 0; j < size; j++) {
            digitAry[i] += *(*(digitInfoAry + j) + i);
        }
    }

    for (i = 0; i < MAX_NUM_DIGITS; i++) {
        if (digitAry[i] == 1) {
            uncommonCount++;
        }
    }

    assembledAry = (int*)malloc(uncommonCount * sizeof(int) + 1);
    *assembledAry = uncommonCount;

    if (uncommonCount != 0) {
        for (i = 0, j = 1; i < MAX_NUM_DIGITS; i+=2) {
            if (digitAry[i] == 1) {
                assembledAry[j] = i;
                j++;
            }
        }
        for (i = 1; i < MAX_NUM_DIGITS; i+=2) {
            if (digitAry[i] == 1) {
                assembledAry[j] = i;
                j++;
            }
        }
    }

    free(digitInfoAry);

    return assembledAry;
}

The commented text displays where the program crashes after running the debugger. What am i doing wrong?

John Smith
  • 71
  • 1
  • 5
  • 4
    When you get an error on `free`, the problem has usually occurred much earlier than that. Run valgrind to find out when it actually happens. – Sergey Kalinichenko Feb 03 '16 at 03:52
  • [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Feb 03 '16 at 03:59
  • Free exactly what is returned by malloc - nothing more and nothing less. Also make sure not to corrupt memory by writing after or before the area returned by malloc. To see what's actually happening, try making your own functions (or macros if you prefer) that print the call and, in the case of malloc, the returned pointer. Check that every malloc pointer returned hits free once and only once, and that nothing else goes to free. – ash Feb 03 '16 at 04:24

2 Answers2

0

I expect the problem is here, where you don't allocate enough memory and then later write off the end of the block, invoking undefined behaviour:

assembledAry = (int*)malloc(uncommonCount * sizeof(int) + 1);

You actually want (uncommonCount + 1) * sizeof(int) bytes.

Unrelated, but you also have a leak when freeing digitInfoAry (because you don't free all the allocated pointers it contains), and you are double-freeing userAry just before leaving main.

paddy
  • 60,864
  • 6
  • 61
  • 103
0

If the user inputs a '1' more than once, memory leaks occur

If the user inputs a '2' after previously entering '1', one ERROR occurs

If the user inputs a '2' without having previously entered '1', two ERRORS occur

do {
    switch (menuOption) {
    case 1:
        userAry = (int*)malloc(...);    //may not get executed
        returnedAry = extractUncommonDigit(...); //may not get executed
        break;
    case 2:
        free(userAry);    //can produce ERROR
        free(returnedAry);//can produce ERROR
        break;
    default:
        break;
    }
} while (menuOption != 2);
free(userAry);            //ERROR, tried to free 'userAry' twice

and the function called:

int* extractUncommonDigit(...) {...}
bgarcia
  • 154
  • 6
  • the problems I mentioned are immediately visible with the trimmed down version of your code as I provided it. I did not examine your code for other faults beyond these 3 – bgarcia Feb 03 '16 at 04:15