Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ims_usrloc_scscf: bugfix Contct is removed when old Contct expires #1547

Merged
merged 1 commit into from Jun 1, 2018
Merged

ims_usrloc_scscf: bugfix Contct is removed when old Contct expires #1547

merged 1 commit into from Jun 1, 2018

Conversation

christoph-v
Copy link
Contributor

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.

Pre-Submission Checklist

  • Commit message has the format required by CONTRIBUTING guide
  • Commits are split per component (core, individual modules, libs, utils, ...)
  • Each component has a single commit (if not, squash them into one commit)
  • 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

  • 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:

  • PR should be backported to stable branches
  • Tested changes locally versus release 5.1.0
  • Related to issue #XXXX (replace XXXX with an open issue number)

Description

This is the pull request for branch 5.1, which has been created by "cherry-pick" from the original commit

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.
@miconda
Copy link
Member

miconda commented Jun 1, 2018

Thanks!

@miconda miconda merged commit 7de9b68 into kamailio:5.1 Jun 1, 2018
@thulasizwe-n
Copy link

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;                                                                           

}

@christoph-v
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants