[kamailio/kamailio] Modules: db_redisusrloc for usrloc module (#1446)
<!-- 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 --> - [ ] 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: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [ ] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number) #### Description <!-- Describe your changes in detail --> You can view, comment on, or merge this pull request online at: https://github.com/kamailio/kamailio/pull/1446 -- Commit Summary -- * kamailio db_redisusrloc -- File Changes -- A src/modules/db_redisusrloc/Makefile (36) A src/modules/db_redisusrloc/README (88) A src/modules/db_redisusrloc/db_redisusrloc.c (100) A src/modules/db_redisusrloc/doc/Makefile (4) A src/modules/db_redisusrloc/doc/db_redisusrloc.xml (45) A src/modules/db_redisusrloc/doc/db_redisusrloc_admin.xml (86) A src/modules/db_redisusrloc/redisdb_base.c (1189) A src/modules/db_redisusrloc/redisdb_connection.c (100) A src/modules/db_redisusrloc/redisdb_connection.h (43) A src/modules/db_redisusrloc/redisdb_dbase.h (96) -- Patch Links -- https://github.com/kamailio/kamailio/pull/1446.patch https://github.com/kamailio/kamailio/pull/1446.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446
@miconda can we have suggestion ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-366944865
I didn't have the time to review yet. Probably it is better that I open a discussion on the mailing list to speed up things by getting more feedback quickly. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-367017248
@miconda are we going to merge this? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-386507713
There was a discussion on mailing lists (sr-dev and sr-users) started in February with the subject "RFC: db_redisusrloc module - review and how to deal with" - can you follow up on that thread, some people had comments and it is better that you express your point of view. I haven't checked yet, but there were some comments/reviews on code for initial PR #1422, have you dealt with them in this PR? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-386786987
sure . Its long back pull request. i need to check whether those comments incorporated or not. On Sat, May 5, 2018 at 1:07 PM, Daniel-Constantin Mierla < notifications@github.com> wrote:
There was a discussion on mailing lists (sr-dev and sr-users) started in February with the subject "RFC: db_redisusrloc module - review and how to deal with" - can you follow up on that thread, some people had comments and it is better that you express your point of view.
I haven't checked yet, but there were some comments/reviews on code for initial PR #1422 <https://github.com/kamailio/kamailio/pull/1422>, have you dealt with them in this PR?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/kamailio/kamailio/pull/1446#issuecomment-386786987>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGb9UQObnq5JAKSibvEUuUEj7t9ivmohks5tvVaqgaJpZM4SJS1k> .
-- With Regards, Surendra Tiwari VoIP Application Developer, Plivo Comm +91-9967609476 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-386787235
@SurendraPlivo pushed 1 commit. ad59363 snprintf issues are fixed -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1446/files/7d6236c3d6e37a27811e464...
@miconda please merge it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-404181127
Just looked quickly a bit ... I spotted some sprintf() which are unsafe, especially when dealing with DB string or blobs, the target buffer is 255 bytes in size, but I haven't seen any check of input size. This snippet needs some checks as well, for allocated pointer and snprintf: ``` + int username_size=VAL_STR(tval).len+1*sizeof(char); + username = (char*)pkg_malloc(username_size); + snprintf(username,username_size,"%s",VAL_STR(tval).s); ``` `pkg_strdup()` can return NULL, but that is not checked -- although, I didn't looked more to see if it always safe to work further if the return is NULL there. I will ask to see if anyone else can do additional work to review. If not, as I said, I do not have anything against merging it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-404219576
@miconda sure. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-404229100
@SurendraPlivo pushed 1 commit. 1999a526ca4a9f57a480121601ba62300f632f1d db_redis: adding the error block -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1446/files/ad59363f9cf91e197fb93d3...
@SurendraPlivo pushed 1 commit. e629543c6540bccb6118d9f55accd08a30edb46c handling snprintf issues, adding table name size as symbolic constant -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1446/files/1999a526ca4a9f57a480121...
Does this one still make sense to merge? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-546318559
@miconda we can close this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-550275696
@SurendraPlivo - thanks for the feedback. So do you switched to the generic redis module or still use this on internally? For reference, it was discussed at this [thread](https://lists.kamailio.org/pipermail/sr-users/2018-February/100423.html). Closed -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#issuecomment-550283703
Closed #1446. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1446#event-2775084590
participants (4)
-
Daniel-Constantin Mierla -
Henning Westerholt
-
Surendra Tiwari
-
surendrasignalwire