Creating a int * with malloc and then use realloc when full

Question

In a function I created am I trying to allocate a int array dynamically to store some index values. First I create the int * with the malloc function and then let the loop store som values in it and increament the pointer each time. The problem I run in to starts when I try to use the realloc to increase the memory allocation. When I do this VS tells me it runs in to undefined behaviour and breaks the program.

The code looks like this

void showAvailable(CabinHolder *holder, Booking *booking)
{
    system("cls");

    printf("Choose cabin number \n");
    printf("Start week: &d \t End week: %d", booking->rentPeriod[0], booking->rentPeriod[1]);
    printf("------------------------------------------\n");

    int memory = 5;
    int *indexOfCabin = (int *)malloc(sizeof(int)*memory);
    int counter = 1;

    for (int i = 0; i < CABINS; i++)
    {
        if (counter == memory)
        {
            memory *= 2;
            int *expanded = realloc(indexOfCabin, (memory * sizeof(int)));
            indexOfCabin = expanded;
            expanded = NULL;
        }

        if (booking->cabin->typeOfCabin == holder->arrofCabin[i].typeOfCabin)
        {
            printf("%d. \t Cabin with number %d \t cost: %d per week\n", counter, holder->arrofCabin[i].nr, holder->arrofCabin[i].cost);
            counter++;
            indexOfCabin = &i;
            indexOfCabin++;
        }
    }

    free(indexOfCabin);
    system("pause");
}

When I debugg in VS i also se that my pointer indexOfCabin seems to be undefined inside the if statement, which I don't understand. What have I missed here?

have a look at this question: https://stackoverflow.com/q/605845/812912 — Ivaylo Strandjev, Feb 14 '18 at 07:54
`indexOfCabin = &i` throws away the memory you allocated, and puts the address of `i` into the pointer. — user3386109, Feb 14 '18 at 07:56
@user3386109 actually realloc takes care of freeing the old memory if needed: http://en.cppreference.com/w/c/memory/realloc — Ivaylo Strandjev, Feb 14 '18 at 07:58
@user3386109 ah that's completely true now that you point it out. How do I instead store the index eg. 17 in indexOfCabin? — anderssinho, Feb 14 '18 at 07:59
@anderssinho please read the documentation of realloc. The statement is not true — Ivaylo Strandjev, Feb 14 '18 at 07:59
`indexOfCabin` can be used like an array, e.g. `indexOfCabin[counter] = i;`. But `counter` needs to start at 0, and should be incremented after being used. And `indexOfCabin` should not be incremented. — user3386109, Feb 14 '18 at 08:02
@user3386109 ah ofc. I'll try that and comeback if there is any problem. Thanks — anderssinho, Feb 14 '18 at 08:03
Hint (not directly related to your question but it might make your life easier): you should outsource the memory management to another function instead of mixing memory management and program logic in the same function. — Jabberwocky, Feb 14 '18 at 09:43
@MichaelWalz that's is also a good tips. Will break out that part in a function of it's own to make the coder easier to understand :) — anderssinho, Feb 14 '18 at 10:30

anderssinho · Answer 1 · 2018-02-14T09:12:17.557

Okay after some help in the comment section I solved the problem with this edited code segment.

void showAvailable(CabinHolder *holder, Booking *booking)
{
    system("cls");

    printf("Choose cabin number \n");
    printf("Start week: %d \t End week: %d\n", booking->rentPeriod[0], booking->rentPeriod[1]);
    printf("------------------------------------------\n");

    int memory = 5;
    int *indexOfCabin = malloc(sizeof(int)*memory);
    int counter = 1;
    int items = 0;
    int choice = 0;

    for (int i = 0; i < CABINS; i++)
    {
        if (counter-1 == memory)
        {
            memory *= 2;
            indexOfCabin = realloc(indexOfCabin, (memory * sizeof(int)));
        }

        if (booking->cabin->typeOfCabin == holder->arrofCabin[i].typeOfCabin)
        {
            printf("%d. \t Cabin with number %d \t cost: %d per week\n", counter, holder->arrofCabin[i].nr, holder->arrofCabin[i].cost);
            counter++;
            indexOfCabin[items++] = i;
        }
    }
    free(indexOfCabin);
    system("pause");
}

First: Problem was indexOfCabin = &i throws away the memory you allocated, and puts the address of i into the pointer instead of what I wanted to do. Now we store the index in i in the pointer.

Second: indexOfCabin can be used like an array, e.g. indexOfCabin[counter] = i;. But counter needs to start at 0, and should be incremented after being used. And indexOfCabin should not be incremented

You should *never* `realloc` the pointer itself (e.g. `indexOfCabin = realloc(indexOfCabin,...`, you should use a temporary pointer `void *tmp = realloc(indexOfCabin,...`, then check `if (tmp)` to validate the reallocation before assigning `indexOfCabin=tmp;`. Otherwise if `realloc` fails you overwrite your original pointer address with `NULL` causing a memory leak. — David C. Rankin, Feb 14 '18 at 09:12
@DavidC.Rankin that's totally true, will change that in the code — anderssinho, Feb 14 '18 at 09:13
That also means delaying `memory *= 2;` until after you validate `realloc` succeeded, so your `realloc` will look like `void *tmp = realloc (indexOfCabin, (memory * 2 * sizeof *indexOfCabin)); if (!tmp) {/* handle error */ break; }; indexOfCabin = tmp; memory *= 2;` — David C. Rankin, Feb 14 '18 at 09:18
@DavidC.Rankin thanks the solution! And understand why you suggest this. It's now implemented in my code. Will update this post also with the change — anderssinho, Feb 14 '18 at 10:32

Creating a int * with malloc and then use realloc when full

1 Answers1