<!-- 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
@miconda can we have suggestion ?
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.
@miconda are we going to merge this?
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?
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 .
@SurendraPlivo pushed 1 commit.
ad59363 snprintf issues are fixed
@miconda please merge it.
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.
@miconda sure.
@SurendraPlivo pushed 1 commit.
1999a526ca4a9f57a480121601ba62300f632f1d db_redis: adding the error block
@SurendraPlivo pushed 1 commit.
e629543c6540bccb6118d9f55accd08a30edb46c handling snprintf issues, adding table name size as symbolic constant
Does this one still make sense to merge?
@miconda we can close this.
@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
Closed #1446.
-
Daniel-Constantin Mierla -
Henning Westerholt
-
Surendra Tiwari
-
surendrasignalwire