Procedural realloc with pointer to an array

Question

To keep the code tidy (and overcomplicated, as I allways do) I want the main() function to pass a /pointer to an array of int/ to a function. That function will then procedurally expand the array and add new entries that match the conditions.

I have a code that works but for some reason valgrind reports a memory leak (from the main array). And also, I don't like how I have to allocate the array, before I can reallocate it.

#include <iostream>
#include <cstdlib>

int get_divisors(int num_input, int *num_result);

int main()
{
    int num_input, *num_result = (int*)malloc(sizeof(int));
    std::cin >> num_input;

    int num_size = get_divisors(num_input, num_result);

    for (int loop_1 = 0; loop_1 < num_size; loop_1++){
        std::cout << num_result[loop_1] << "\n";
    }

    free(num_result);
}

int get_divisors(int num_input, int *num_result)
{
    int num_count = 0;
    for (int loop_1 = 1; loop_1 < num_input; loop_1++){
        if (num_input % loop_1 == 0){   
            num_count++;
            num_result = (int*)realloc(num_result, num_count*sizeof(int));
            num_result[num_count - 1] = loop_1;
        }
    }

    return num_count;
}

Valgrind: (--leak-check=full --show-leak-kinds=all -v)

==13337== HEAP SUMMARY:
==13337==     in use at exit: 72,724 bytes in 2 blocks
==13337==   total heap usage: 7 allocs, 6 frees, 72,768 bytes allocated
==13337== 
==13337== Searching for pointers to 2 not-freed blocks
==13337== Checked 123,368 bytes
==13337== 
==13337== 20 bytes in 1 blocks are definitely lost in loss record 1 of 2
==13337==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13337==    by 0x400A0A: get_divisors(int, int*) (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337==    by 0x400964: main (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337== 
==13337== 72,704 bytes in 1 blocks are still reachable in loss record 2 of 2
==13337==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13337==    by 0x4E9B2AF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==13337==    by 0x4010139: call_init.part.0 (dl-init.c:78)
==13337==    by 0x4010222: call_init (dl-init.c:36)
==13337==    by 0x4010222: _dl_init (dl-init.c:126)
==13337==    by 0x4001309: ??? (in /lib/x86_64-linux-gnu/ld-2.19.so)
==13337== 
==13337== LEAK SUMMARY:
==13337==    definitely lost: 20 bytes in 1 blocks
==13337==    indirectly lost: 0 bytes in 0 blocks
==13337==      possibly lost: 0 bytes in 0 blocks
==13337==    still reachable: 72,704 bytes in 1 blocks
==13337==         suppressed: 0 bytes in 0 blocks
==13337== 
==13337== ERROR SUMMARY: 7 errors from 3 contexts (suppressed: 0 from 0)
==13337== 
==13337== 1 errors in context 1 of 3:
==13337== Invalid free() / delete / delete[] / realloc()
==13337==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13337==    by 0x4009B7: main (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337==  Address 0x5a37c80 is 0 bytes inside a block of size 4 free'd
==13337==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13337==    by 0x400A0A: get_divisors(int, int*) (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337==    by 0x400964: main (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337== 
==13337== 
==13337== 5 errors in context 2 of 3:
==13337== Invalid read of size 4
==13337==    at 0x400985: main (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337==  Address 0x5a37c80 is 0 bytes inside a block of size 4 free'd
==13337==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13337==    by 0x400A0A: get_divisors(int, int*) (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337==    by 0x400964: main (in /media/areuz/Storage - Linux/1 :: Documents/Programming/2 :: C++ Language/1 :: C++ Codes/Divisibility Check/divisibility)
==13337== 
==13337== ERROR SUMMARY: 7 errors from 3 contexts (suppressed: 0 from 0)

You need a reference int get_divisors(int num_input, int *& num_result) — user2249683
– user2249683, Commented Mar 8, 2016 at 22:49
Also, if you want to code C++ use std::vector get_divisors(int num_input) and never use that ancient realloc — user2249683
– user2249683, Commented Mar 8, 2016 at 22:54
I think one of the first questions on Stack Overflow must have been about realloc. Pretty sure there's at least one dupe from 2007. I'll go look for it. — Zan Lynx
– Zan Lynx, Commented Mar 9, 2016 at 0:52
I'd vote this a dup but what's the point when there are 167 pages of answers that match [c] realloc — Zan Lynx
– Zan Lynx, Commented Mar 9, 2016 at 0:57

Christophe · Accepted Answer · 2016-03-08 23:01:29Z

Problem:

In your get_divisor() function, the following statement reallocates the pointer:

       num_result = (int*)realloc(num_result, num_count*sizeof(int));

but num_result is a parameter passed by value, so you only change this local variable. The pointer in main remains unchanged, so that you will loose the memory reallocated as soon as you return from the function, if memory gets moved to another address.

Quick fix:

You could pass the pointer by reference so that it implements your intended approach (i.e. the original pointer really gets modified within the function). Simply change the function signature/definition:

int get_divisors(int num_input, int *&num_result);  // pass by reference

Better approach:

First, you should really get rid of malloc() and realloc() in c++: these work well with basic types, but do not respect the object lifecycle of more complex objects (i.e. objects are not constructed).

Next, you shouldn't reinvent the wheel. Instead of managing yourself a variable sized array, go for vectors:

vector<int> vnum_result(1);  // initial size is one.  
vnum_result.resize(num_count);  // change the size 
                                // no worries about freeing.

Of course, the question of passing by reference or by value will still remain.

Thanks for the quick answer, now It works, and I'll look into getting rid of malloc tomorrow. As you probably can tell, I've just started transitioning into c++ (from c of course).

user2249683 · Accepted Answer · 2016-03-09 00:21:21Z

You should get used to C++ containers and not being afraid of returning a container (lookup return value optimization and copy elision):

#include <iostream>
#include <vector>

std::vector<int> get_divisors(int num_input)
{
    std::vector<int> result;
    for (int loop_1 = 1; loop_1 < num_input; loop_1++) {
        if (num_input % loop_1 == 0) {
            result.push_back(loop_1);
        }
    }
    return result;
}

int main()
{
    int num_input;
    if(std::cin >> num_input) {
        std::vector<int> divisors = get_divisors(num_input);
        // You may consider a ranged base for: for (int divisor : divisors)
        for (std::size_t loop_1 = 0; loop_1 < divisors.size(); loop_1++){
            std::cout << divisors[loop_1] << "\n";
        }
    }
}

Collectives™ on Stack Overflow

Procedural realloc with pointer to an array

2 Answers 2

1 Comment

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Linked

Related