[kamailio/kamailio] dmq_usrloc: DMQ also copies and sends attributes related to contacts (PR #3679)
<!-- 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 --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail --> This PR fixes the issue in https://github.com/kamailio/kamailio/issues/1968.
Any feedback is welcome. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3679
-- Commit Summary --
* dmq_usrloc: Transfer attributes * usrloc: Modify destroy order and contact xavp
-- File Changes --
M src/modules/dmq_usrloc/usrloc_sync.c (132) M src/modules/usrloc/ucontact.c (9)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3679.patch https://github.com/kamailio/kamailio/pull/3679.diff
@miconda commented on this pull request.
@@ -825,6 +837,16 @@ int usrloc_dmq_send_contact(
srjson_AddNumberToObject(&jdoc, jdoc.root, "reg_id", ptr->reg_id); srjson_AddNumberToObject(&jdoc, jdoc.root, "server_id", ptr->server_id);
+ /* Loop through Χavp attributes of the contact and and create a json object */ + srjson_t *jdoc_xavp = srjson_CreateObject(&jdoc);
Variable should be declared at the beginning of the block to be coherent and also keep compliant with older C standards.
The commit for usrloc #3d79568 does not only change the destroy order, is it also fixing something?
@xkaraman pushed 2 commits.
a192e503f58507fdca4dc5cb861a4dc309873e81 dmq_usrloc: Transfer attributes 6673124597eb94bcf00d81c5e2598d4eee7ee832 usrloc: Modify destroy order and contact xavp
@xkaraman commented on this pull request.
@@ -825,6 +837,16 @@ int usrloc_dmq_send_contact(
srjson_AddNumberToObject(&jdoc, jdoc.root, "reg_id", ptr->reg_id); srjson_AddNumberToObject(&jdoc, jdoc.root, "server_id", ptr->server_id);
+ /* Loop through Χavp attributes of the contact and and create a json object */ + srjson_t *jdoc_xavp = srjson_CreateObject(&jdoc);
Fixed on some other variables too on the newly static functions.
Hey Daniel,
Besides the order change of destroy, it copies to the new contact the `xavp` from incoming `ucontact_t` argument just like the rest of the fields.
If the config variable `xavp_contact` is found and not empty, then `ucontact_xavp_store` destroy this xavp and fills it up with whatever is contained inside.
Some doubts i had, is whether i shall `c->xavp = _ci->xavp;` or ` _c->xavp = xavp_clone_level_nodata(_ci->xavp);` or `xavp_add(_ci->xavp, &c->xavp)`. What is the difference between all these methods?
Are there more comments or objections? If not I would merge it in the next days.
I haven't gotten the time to review if the list of XAVPs can be just linked or needs to be cloned. If it is just used temporarily in a function, then linking should be fine, but if it needs to be kept for long, then it could be that clone has to be done, otherwise the list stays linked from two places and attempted to be destroyed twice.
If the above is clarified, I am fine to merge it.
Thanks, yes this was basically the question also @xkaraman had earlier. The XAVPs will be used in the cfg script context later on I think. He will do another review.
Hi @miconda,
What does long mean in this case? What does `xavp_clone_level_nodata` achieve in this regard?
Thanks!
@xkaraman: I meant to be kept for "long time" -- for example, if the list is needed for the duration of keeping a contact record till its expiration (or, just as another example, for dialog record till its end). In such case, the list has to be cloned, or the pointer to it from the core (or other structures) has to be reset, to avoid being linked from two places, because it can be destroyed from one place and still referenced from the other place.
@xkaraman pushed 2 commits.
b6e9fe81e4464d462dfb3b0dc5f469565cd63b1e dmq_usrloc: Transfer attributes 709fa172980f018a9ae2b502f2d3bdef531c3080 usrloc: Modify destroy order and contact xavp
@xkaraman pushed 2 commits.
b4fbf7f9d120f835be8d906a3ac62e7de14f2e07 dmq_usrloc: Transfer attributes c57e1f21f35145ba6df93ea5db2d0a8ebe2ff36c usrloc: Modify destroy order and contact xavp
Closed #3679.
The PR was reviewed and discussed together with Xenofon, some comments have been added and it was pushed in the referenced commits. Close this PR.
-
Daniel-Constantin Mierla -
Henning Westerholt
-
Xenofon Karamanos