0

I am facing memory leak problem with the below code

static char **edits1(char *word)
{
    int next_idx;
    char **array = malloc(edits1_rows(word) * sizeof (char *));
    if (!array)
        return NULL;

    next_idx = deletion(word, array, 0);
    next_idx += transposition(word, array, next_idx);
    next_idx += alteration(word, array, next_idx);
    insertion(word, array, next_idx);

    return array;
}

static void array_cleanup(char **array, int rows) {

        int i;

        for (i = 0; i < rows; i++)
            free(array[i]);
}

static char *correct(char *word,int *count) {

        char **e1, **e2, *e1_word, *e2_word, *res_word = word;
        int e1_rows, e2_rows,max_size;

        e1_rows = edits1_rows(word);
        if (e1_rows) {
            e1 = edits1(word);
        *count=(*count)*300;
            e1_word = max(e1, e1_rows,*count);

            if (e1_word) {

                array_cleanup(e1, e1_rows);
                        free(e1);
                return e1_word;

            }
        }

    *count=(*count)/300;

    if((*count>5000)||(strlen(word)<=4))
        return res_word;

        e2 = known_edits2(e1, e1_rows, &e2_rows);
        if (e2_rows) {
        *count=(*count)*3000;
            e2_word = max(e2, e2_rows,*count);
            if (e2_word)
                    res_word = e2_word;
        }

        array_cleanup(e1, e1_rows);
        array_cleanup(e2, e2_rows);

        free(e1);
        free(e2);
        return res_word;
}

I don’t know why free() is not working. I am calling this function "correct" in thread, multiple threads are running simultaneously.I am using Ubuntu OS.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
Rahul
  • 1
  • 2
  • Where your free function is not working in the code ? If its not working anywhere, have you considered including malloc.h – krammer Oct 25 '12 at 07:29
  • 3
    Have you tried tools such as [Valgrind](http://valgrind.org/) to find the leaks/memory problems? – Some programmer dude Oct 25 '12 at 07:31
  • @krammer i didn`t include malloc.h , is it really important? my program is working well except memory leak issue – Rahul Oct 25 '12 at 07:41
  • That's some weird code you've got there. Why are you (optionally) multiplying by 300 and then dividing by 300? Consider `#define`ing some constants. – Zecc Oct 25 '12 at 07:41
  • 1
    AFAIR free() can only deallocate memory that was allocated by *alloc() functions. I don't see any code that allocates memory... – Germann Arlington Oct 25 '12 at 07:42
  • @JoachimPileborg i had used valgring , but its not helping me, while running valgrind, memory overflow occurs – Rahul Oct 25 '12 at 07:42
  • @GermannArlington static char **edits1(char *word) { int next_idx; char **array = malloc(edits1_rows(word) * sizeof (char *)); if (!array) return NULL; next_idx = deletion(word, array, 0); next_idx += transposition(word, array, next_idx); next_idx += alteration(word, array, next_idx); insertion(word, array, next_idx); return array; } this function returns an allocated array to the calling function – Rahul Oct 25 '12 at 07:44
  • What do you mean with "not working"? You'd have more problems on your system if free wasn't working, so I'd say that free is working fine, it's just that you misdiagnosed the problem. – Art Oct 25 '12 at 07:46
  • @krammer What is malloc.h? I have never heard of it, it is not a standard C library header. The function `malloc` is found in stdlib.h. – Lundin Oct 25 '12 at 07:47
  • What do you mean by "memory overflow"? That, together with or problem of not being able to free (how do you know that?) might be a symptom of a deeper error in your code, overwriting arrays or using unallocated memory or some such. – Some programmer dude Oct 25 '12 at 07:48
  • @Art but the allocated memory is not being deallocated , i am tracking memory usage using htop command in ubuntu, it keeps on increasing on each call – Rahul Oct 25 '12 at 07:50
  • Most malloc implementations usually don't return the memory to the operating system, but rather keep it for future calls to malloc. Returning the memory to the operating system impacts performance quite a lot. If you have certain allocation patterns, the memory that malloc keeps for future use can't be reused by malloc and that's called memory fragmentation. Whatever htop reports is not how much memory you have allocated with malloc, but all the various allocations that all libraries did, which could be much much more than you're using. – Art Oct 25 '12 at 07:53
  • @JoachimPileborg my program is working well , output and all are correct , only issue is with the memory, if i just comment that function(correct()) , memory usage doesn`t increase – Rahul Oct 25 '12 at 07:53
  • What are you trying to do here `char **array = malloc(edits1_rows(word) * sizeof (char *));`? – Germann Arlington Oct 25 '12 at 07:54
  • @GermannArlington allocating memory for storing all 1-edit distance words generated from the word pointed by "word" – Rahul Oct 25 '12 at 08:01
  • Now take a look at what you are actually doing: what do you expect the `sizeof (char *)` to return? What is the result of `edits1_rows(word)`? Now, what is `malloc()` going to attempt to allocate? – Germann Arlington Oct 25 '12 at 08:13
  • @Lundin ah, I was just referring that he might have missed to include the function since it was written that "free" is not working. – krammer Oct 25 '12 at 08:23

2 Answers2

0

You don't show where you allocate the actual arrays, you just show where you allocate the array of pointers. So it is quite possible that you have leaks elsewhere in the code you are not showing.

Furthermore, array_cleanup leaks since it only deletes those arrays you don't show where you allocate. It doesn't delete the array of pointers itself. The final row of that function should have been free(array);.

Your main problem is that you are using an obscure allocation algorithm. Instead, allocate true dynamic 2D arrays.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • char **array = malloc(edits1_rows(word) * sizeof (char *)); i am allocating array using the above code – Rahul Oct 25 '12 at 08:13
  • @user1768256 On that line you are _only_ allocating an array of _pointers_, you are not allocating any actual _data_. And you free it incorrectly, as I pointed out, which is why you have a memory leak. – Lundin Oct 25 '12 at 09:04
0

Answer based on digging for further information in comments.

Most malloc implementations usually don't return the memory to the operating system, but rather keep it for future calls to malloc. This is done because returning the memory to the operating system can impact performance quite a lot.

Furthermore, if you have certain allocation patterns, the memory that malloc keeps might not be easily reusable by future calls to malloc. This is called memory fragmentation and is a large topic of research for designing memory allocators.

Whatever htop/top/ps reports is not how much memory you have currently allocated inside your program with malloc, but all the various allocations that all libraries did, their reserves and such, which could be much more than you've allocated.

If you want an accurate assessment of how much memory you are leaking, you need to use a tool like valgrind or see if maybe the malloc you're using has diagnostic tools to help you with that.

Art
  • 19,807
  • 1
  • 34
  • 60