Fusionmate
Fusionmate

Reputation: 689

Will returning an array from a function cause a memory leak?

Since we cannot free the local pointer "tmparr" which defining in the function, will it cause memory leak? Not sure is it any better coding solution here.

I am trying to pass a pointer to a function, so that it will process and change the data inside the function as well. The size of the data and value can flexible modify within the function.

void printArray(int* data, 
                int length)
{
    for (int i=0; i<length; i++) {

        if ((i>0) && (i%5==0))
            printf("\n");

        printf("%d ", data[i]);
    }

    printf("\n\n");

}

void copyPassPtrArray(int** data,
                      int length)
{
    int* tmparr = (int*)malloc(length * sizeof(int));

    for (int i=0; i<length; i++)
        tmparr[i] = i + 10;

    *data = tmparr;
}


int main()
{
    int length = 10;

    int* ptrarr = (int*)malloc(length * sizeof(int));

    for (int i =0; i <length; i++)
        ptrarr[i] = i;

    printf("Origin ... \n");
    printArray(ptrarr, length);

    copyPassPtrArray(&ptrarr, 20);
    printf("After copyPassPtrArray ... \n");
    printArray(ptrarr, 20);

    free(ptrarr);

    return 0;
}

After reading the comments, I have the following proposed api solution. The purpose of this practice is that we may not know what will the size of an array and values after certain computation in the function. It "data" need to return back to main or other function. However, is there anymore memory leak?

void copyGlobalPtrArray(int** data,
                        int length)
{   
    *data = (int*)malloc(length * sizeof(int));

    for (int i=0; i<length; i++)
        (*data)[i] = i + 10;
}

Upvotes: 0

Views: 1100

Answers (3)

Sanjay Manohar
Sanjay Manohar

Reputation: 7026

Yes there is a leak. But it's not where you think it is.

You have correctly free'd the area allocated in copyPassPtrArray. However, the originally allocated pointer,

int* ptrarr = (int*)malloc(length * sizeof(int));

has not been freed. That is because you over-wrote the original data's pointer in the line

copyPassPtrArray(&ptrarr, 20);

The old pointer is lost forever! Instead, you should store into a new pointer, and free both pointers at the end.

e.g.

int main()
{
    int length = 10;

    int* ptrarr = (int*)malloc(length * sizeof(int);
    int* copyarr;

    for (int i =0; i <length; i++)
        ptrarr[i] = i;

    printf("Origin ... \n");
    printArray(ptrarr, length);

    copyPassPtrArray(&copyarr, 20);     // put copied pointer into a separate variable
    printf("After copyPassPtrArray ... \n");
    printArray(copyarr, 20);

    free(ptrarr);
    free(copyarr);
    return 0;
}

Upvotes: 3

Siddesh Parmar
Siddesh Parmar

Reputation: 103

If you are not too sure whether you have written code which is free from memory leak problem then its a good practice to use memory leak tool

valgrind is one the tool which can be used .

below is the command :

valgrind --tool=memcheck --leak-check=full  ./memleak  (program name)
==422== Command: ./memleak
Origin ... 
0 1 2 3 4 
5 6 7 8 9 
After copyPassPtrArray ...
10 11 12 13 14 
15 16 17 18 19 
20 21 22 23 24 
25 26 27 28 29 

==422== HEAP SUMMARY:
==422==     in use at exit: 40 bytes in 1 blocks
==422==   total heap usage: 2 allocs, 1 frees, 120 bytes allocated  // here total allocations n free done  in ur code

==422== 40 bytes in 1 blocks are definitely lost in loss record 1 of 1
==422==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==422==    by 0x400767: main (memleak.c:35)    //shows line number 

==422== LEAK SUMMARY:
==422==    definitely lost: 40 bytes in 1 blocks  // here u can see memory leak
==422==    indirectly lost: 0 bytes in 0 block
==422==      possibly lost: 0 bytes in 0 blocks
==422==    still reachable: 0 bytes in 0 blocks
==422==         suppressed: 0 bytes in 0 blocks

==422== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

So its better to run this and try to solve memory leak yourself

Upvotes: 3

user707650
user707650

Reputation:

Usually, it's not a good idea to allocate memory inside a function that is not freed inside the same function. Better, allocate the memory for a variable outside the function, pass the variable to the function, and free the variable outside the function as well.

Something like this:

void copyPassPtrArray(int *tmparr, int length)
{
    for (int i=0; i<length; i++)
        tmparr[i] = i + 10;
}


int main()
{
    int length = 10;
    int doublelength = 2 * length;

    int* ptrarr = (int*)malloc(length * sizeof(int));
    int* newarr = (int*)malloc(doublelength * sizeof(int));

    for (int i =0; i <length; i++)
        ptrarr[i] = i;

    ...

    copyPassPtrArray(newarr, doublelength);

    ...

    free(newarr);
    free(ptrarr);

    return 0;
}

If you want to allocate memory inside a function, explicitly return that memory in a variable, and assign it to a (new) variable.

Then your code could be something like:

int *copyPassPtrArray(int length)
{
    int* tmparr = (int*)malloc(length * sizeof(int));

    for (int i=0; i<length; i++)
        tmparr[i] = i + 10;

    return tmparr;
}


int main()
{
    int length = 10;

    int* ptrarr = (int*)malloc(length * sizeof(int));
    int* newarr = NULL;

    for (int i =0; i <length; i++)
        ptrarr[i] = i;

    ...

    newarr = copyPassPtrArray(20);

    ...

    free(newarr)
    free(ptrarr);

    return 0;
}

If you now would have the line ptrarr = copyPassPtrArray(20); instead, you would easier notice you're overriding/reassigning ptrarr, since it's in the same block as its earlier allocation.

Upvotes: 2

Related Questions