The pileup interface maintains a linked list of lbnode_t pointers. These start at iter->head, chain through lbnode->next, and then end up at iter->tail. We also have a separate linked list in iter->mp of items we've previously freed, so we don't have to free and malloc continually.
bam_plp64_next adds and removes items to these linked lists, and it calls iter->plp_destruct when it puts items into the free list. So far so good, and so correct.
However if for whatever reason we bail out of the pileup interface early, before we've punted all records onto the iter->mp free list, then we weren't calling plp_destruct on the current "in flight" data. This caused a memory leak, fixed in d028e0d.
Unfortunately there is a subtlety I didn't notice at the time. The in-flight linked list goes from iter->head to one before iter->tail. The tail is simply a dummy node and unused by the code. I don't understand why it has to work this way, but presumably someone didn't want iter->head and iter->tail to ever point to the same item.
The bam_plp_destroy function however has to move all these items to the iter->mp free list, so here it goes from iter->head to iter->tail inclusively. This commit avoids attempting to call the destructor on the tail, which could be a previously freed item that was pulled back off the iter->mp list, leading to double frees.
The pileup interface maintains a linked list of lbnode_t pointers. These start at iter->head, chain through lbnode->next, and then end up at iter->tail. We also have a separate linked list in iter->mp of items we've previously freed, so we don't have to free and malloc continually.
bam_plp64_next adds and removes items to these linked lists, and it calls iter->plp_destruct when it puts items into the free list. So far so good, and so correct.
However if for whatever reason we bail out of the pileup interface early, before we've punted all records onto the iter->mp free list, then we weren't calling plp_destruct on the current "in flight" data. This caused a memory leak, fixed in d028e0d.
Unfortunately there is a subtlety I didn't notice at the time. The in-flight linked list goes from iter->head to one before iter->tail. The tail is simply a dummy node and unused by the code. I don't understand why it has to work this way, but presumably someone didn't want iter->head and iter->tail to ever point to the same item.
The bam_plp_destroy function however has to move all these items to the iter->mp free list, so here it goes from iter->head to iter->tail inclusively. This commit avoids attempting to call the destructor on the tail, which could be a previously freed item that was pulled back off the iter->mp list, leading to double frees.