Potential memory leaks in slist functions about collections-c HOT 6 CLOSED

Ok, accepted. Await another PR soon :)

from collections-c.

Awesome :)

from collections-c.

I don't think it's much of a problem since the user would have to pass an out to slist_iter_next anyway and would thus have a reference to free.

...
void *e;
while (slist_iter_next(iter, &e) != CC_ITER_END) {
    ...
    slist_iter_remove(iter, NULL); // Even though the output is ignored the user still has
                                   // access to the removed reference within the iterator context
}

Basically iter_remove removes the element that was already returned by the iter_next and it can't operate on any other value so it should be safe to ignore its out parameter.

from collections-c.

Maybe I'm missing something... but it seems to me that the caller cannot access the data of the removed element due to these lines:

    void *e = unlink(iter->list, iter->current, iter->prev);
    iter->current = NULL;

The only option to store the pointer to data is to provide 'out'.

from collections-c.

Basically, I started digging into this because valgrind is reporting memory leaks when running 'slist_test'. Some of them are internal to the test code, but others lead to the functions like slist_iter_remove.

from collections-c.

Technically true because after you call iter_remove you can no longer access the data from the list, but in practice iter_next gives you the data pointer before you get a chance to remove it from the list. iter_remove's out just returns the same thing that iter_next out returned and that's why it's not really a big deal if you ignore it or not.

I also ran valgrind and it seems that the problems are actually coming from slist_1234, or to be more precise not freeing its malloced data properly. Usually this list is freed by calling slist_destroy_free which also calls free on the data (and sometimes it's not freed at all :-), but since some of the malloced values are removed from the list, slist_destroy_free can't reach them. This is why the problem mostly comes from tests that call remove.

This can be fixed by manualy freeing the data after the remove:

    int *e;
    while (slist_iter_next(&iter, (void*) &e) != CC_ITER_END) {
        if (*e == 3) {
            slist_iter_remove(&iter, NULL);
            free(e);  // don't let data float around in interdimensional space
        }
    }

from collections-c.