c free temporary pointer to struct

Question

I have a small problem in c, i sort some data, with a bubble sort, (change a <=> b with a tmp) Let's see some code, you'll understand.

void ft_sort_dico(t_dico **dico)
{
    int y;
    t_dico *tmp;

    tmp = (t_dico *)malloc(sizeof(t_dico));
    y = 0;
    while (dico[y])
    {
        if (dico[y + 1] && ft_strcmp(dico[y]->key, dico[y + 1]->key) > 0)
        {
            tmp = dico[y];
            dico[y] = dico[y + 1];
            dico[y + 1] = tmp;
            y = -1;
        }
        y++;
    }
    free(tmp); <- error
}

You haven't said what the error is, so I'm going to assume it's some sort of memory corruption and tell you to apply [`valgrind`](http://valgrind.org/). — zwol, Aug 28 '15 at 14:01
[Please don't cast the result of `malloc()`](http://stackoverflow.com/a/605858/3233393). — Quentin, Aug 28 '15 at 14:34
Remove statements tmp = (t_dico *)malloc(sizeof(t_dico)); and free(tmp); The code will work. As you donot need malloc/free at all. You are just trying to use tmp to swap pointers. — Ritesh, Aug 28 '15 at 14:47
To know what you have allocated (or attempted to allocate), you need to show the definition of `t_dico` what is presumably a struct or typedef... — David C. Rankin, Aug 28 '15 at 15:01
the assignment statements only copy the pointer, not the whole t_diso struct. (which works correctly for your algorithm). so no need to ever allocate memory for the `tmp` pointer — user3629249, Aug 30 '15 at 13:15
Please do some reading as to what `malloc()` and `free()` is for. — The Paramagnetic Croissant, Aug 30 '15 at 13:29

score 1 · Answer 1 · answered Aug 28 '15 at 14:01

1

You free another pointer than you malloc -- at the point your reach free, tmp points to dico[something], which is definitely not the memory you allocated at the beginning.

answered Aug 28 '15 at 14:01

Marcus Müller

34,677
4
53
94

Yeah Marcus i understood what you mean ... i freed the pointed value, not the new one.. So i add a new variable, after malloc who knows the adress of the original one. – albttx Aug 28 '15 at 14:08
1

@RedWine, there is no/zero need for the malloc/free as only pointers are being manipulated during the sort operation, not the whole contents of the structs – user3629249 Aug 30 '15 at 13:24

score 1 · Answer 2 · answered Aug 28 '15 at 14:26

1

tmp = dico[y]; Here you change the address to which tmp points at. You create a memory leak and the program will crash & burn when you call free.

answered Aug 28 '15 at 14:26

Lundin

195,001
40
254
396

Further down, he also does `dico[y+1] = tmp` followed by `free(tmp)` which means that `dico` contains dangling pointers all over the place. – Nik Bougalis Aug 28 '15 at 14:39

user3629249 · Accepted Answer · 2015-08-30T13:20:04.693

given this posted code:

tmp = (t_dico *)malloc(sizeof(t_dico));
y = 0;
while (dico[y])
{
    if (dico[y + 1] && ft_strcmp(dico[y]->key, dico[y + 1]->key) > 0)
    {
        tmp = dico[y];
        dico[y] = dico[y + 1];
        dico[y + 1] = tmp;
        y = -1;
    }
    y++;
}
free(tmp);

the tmp initially gets a pointer to some allocated memory, via the call to malloc()

Then this line overlays that pointer:

tmp = dico[y];

the result is a memory leak

Then the code passes a pointer to one of the elements in the dico array (the one from dico[y]) to the free() function.

to correct this, remove the statement that calls malloc() and remove the statement that calls free().

BTW: this sort algorithm does not actually perform the desired sort. Suggest implementing a bubble or insertion or selection sort.

here is the algorithm for a selection sort.

Where 'n' is number of entries in array[]

for ( c = 0 ; c < ( n - 1 ) ; c++ )
{
    position = c;

    for ( d = c + 1 ; d < n ; d++ )
    {
       if ( array[position] > array[d] )
         position = d;
    }
    if ( position != c )
    {
       temp = array[c];
       array[c] = array[position];
       array[position] = temp;
    }
}

score -1 · Answer 4 · answered Aug 28 '15 at 14:33

Since you do not want to store the tmp pointer, you can just define tmp as a local variable and it would be stored at stack and do not require your manual free.

When you call malloc(), you specify the block size that would be allocated, while it actually use a slightly more space recording the block size information(and maybe something else). This is why when you call free(), the function know how much to free.

In your case, when you call free(tmp), as long as the statement dico[y + 1] && ft_strcmp(dico[y]->key, dico[y + 1]->key) > 0 become TRUE for once, the pointer is no longer pointed to the memory you allocate before (would be some dico[y]). The free() function do not know how mch to free thus return an error.

`tmp` needs to be a pointer, as only the pointers are being manipulated, not the whole contents of the structs. So, while this 'fix' would eliminate the current problem, it would raise a new one. — user3629249, Aug 30 '15 at 13:25

c free temporary pointer to struct

4 Answers4