Osmocom Libraries » libosmocore

There is a problem if the item enqueued in an osmo_it_q has its llist_head anywhere else than right at the start:
_osmo_it_q_flush() will crash at talloc_free(item).

This works:

struct my_item {
   struct llist_head entry;
   int foo;
}

struct my_item *i = my_item_alloc();
osmo_it_q_enqueue(my_queue, i, entry);
osmo_it_q_flush(my_queue);

because &entry coincides with the talloc allocated struct my_item *.

This fails:

struct my_item {
   int foo;
   struct llist_head entry;
}

struct my_item *i = my_item_alloc();
osmo_it_q_enqueue(my_queue, i, entry);
osmo_it_q_flush(my_queue);

because &entry is not pointing at the right place for talloc to work.

So there is no point in having both the item and the member arg for osmo_it_q_enqueue(). They have to be the same pointer anyway. It raises the false impression that the llist entry may sit anywhere in my_item.

I think the freeing part should be left to the caller entirely, like with osmo_it_q_dequeue(). osmo_it_q_flush() is the only function that talloc_free()s a pointer from the caller.

my humble suggestions, one of

• deprecate osmo_it_q_flush() entirely -- the caller has to loop over osmo_it_q_dequeue() instead, and do each free properly.
  osmo_it_q_destroy() would then require the queue to be empty instead of calling flush.
• change osmo_it_q_flush() to take a callback function arg to free each item. (destroy would also need this cb arg)
• add a freeing callback to the struct osmo_it_q definition.

We could also instead publicly require in the API doc that the llist member is at the start of the item struct, and remove the third useless argument from the enqueue function.