in function unlink_contact_from_impu() (in file impurecord.c) an assignment "=" is used instead of a comparision operator "==". This leads to mess, when old contact expires.
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [x] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [x] PR should be backported to stable branches - [x] Tested changes locally versus release 5.1.0 - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail -->
This is the pull request for branch 5.1, which has been created by "cherry-pick" from the original commit
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1547
-- Commit Summary --
* ims_usrloc_scscf: bugfix Contct is removed when old Contct expires
-- File Changes --
M src/modules/ims_usrloc_scscf/impurecord.c (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1547.patch https://github.com/kamailio/kamailio/pull/1547.diff
Thanks!
Merged #1547.
Hi @christoph-v Good fix...i see it's revealing another two potential issues on remove_impucontact_from_list same file (impurecord.c); 1) tail->next is not nullified on removing last impucontact--hence second last will still point to the "just-removed" impucontact 2) contact removal in mid list seems to have swapped fields on both sides of equal sign Please see below possible fix:
int remove_impucontact_from_list(impurecord_t* impu, impu_contact_t *impucontact) { ucontact_t* contact = impucontact->contact;
if (/*contact - removed*/ impucontact == impu->linked_contacts.head /*->contact - removed */) { LM_DBG("deleting head\n"); impu->linked_contacts.head = impu->linked_contacts.head->next; } else if (/*contact*/ impucontact== impu->linked_contacts.tail/*->contact*/) { LM_DBG("deleting tail prev %p next %p\n",impu->linked_contacts.tail->prev, impu->linked_contacts.tail->next); impu->linked_contacts.tail = impu->linked_contacts.tail->prev; impu->linked_contacts.tail->next = 0; /* Nullified end of list*/ } else { LM_DBG("deleting mid list prev %p next %p\n", impucontact->prev, impucontact->next); impucontact->prev->next = impucontact->next; /*impucontact->prev = impucontact->next->prev; removed ---- seems to be small swap-around error*/ impucontact->next->prev = impucontact->prev; /* Added */ } impu->linked_contacts.numcontacts--; if (impucontact->contact->is_3gpp) impu->linked_contacts.num3gppcontacts--; LM_DBG("REMOVED impu_contact %p for contact %p\n",impucontact, contact); shm_free(impucontact); return 0; }
Hi @thulasizwe-n Thank you for the hint.
We will issue a pull request for this additional issue within the next few days, @miconda OK?