6 Jan
2020
6 Jan
'20
5:06 p.m.
- fixed dynamic sources loading
- fixed match_mode in check_whitelist function
<!-- 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
- [ ] 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/2190
-- Commit Summary --
* Added missing KEMI functions, fixed sources loading and match_mode in
-- File Changes --
M src/modules/userblacklist/userblacklist.c (173)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2190.patch
https://github.com/kamailio/kamailio/pull/2190.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/2190
6 Jan
6 Jan
5:14 p.m.
Thanks for the contribution! The commit message should be formatted according to the
contributing guide:
*
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md#co…
You actually did the pull request title and comments like the commit message should have
been.
This PR can be eventually merged manually with the proper commit message format, but have
it in mind for future contributions.
I have also some comment to the code of the commit, otherwise maybe @henningw or
@lbalaceanu would like to review before merging.
--
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/2190#issuecomment-571225562
5:18 p.m.
miconda commented on this pull request.
+ LM_ERR("no table name\n");
+ return -1;
+ }
+ /* try to add the table */
+ if (add_source(table) != 0) {
+ LM_ERR("could not add table");
+ return -1;
+ }
+
+ gnode = table2dt(table);
+ if (!gnode) {
+ LM_ERR("invalid table '%s'\n", table);
+ return -1;
+ }
+
+ arg = pkg_malloc(sizeof(struct check_blacklist_fs_t));
The `arg` is allocated in pkg, shouldn't it be freed after the call of
`check_blackist()`? There are similar cases in the other functions. I am not familiar with
the code of the module, but upon quick check there are at least some error cases inside
`check_blackist()` that just return back without any pkg free.
--
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/2190#pullrequestreview-338775644
5:29 p.m.
@fsantulli pushed 1 commit.
d5294563c3be9bcc7e21579c1fdf59af2e3a8cc0 Added pkg_free at returning of check_whitelist
and check_blacklist
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2190/files/e354d41e12ad0cabb63e84…
5:30 p.m.
fsantulli commented on this pull request.
+ LM_ERR("no table name\n");
+ return -1;
+ }
+ /* try to add the table */
+ if (add_source(table) != 0) {
+ LM_ERR("could not add table");
+ return -1;
+ }
+
+ gnode = table2dt(table);
+ if (!gnode) {
+ LM_ERR("invalid table '%s'\n", table);
+ return -1;
+ }
+
+ arg = pkg_malloc(sizeof(struct check_blacklist_fs_t));
Hi Daniel, i've just added the missing pkg_free before the functions returns. Sorry of
the mistake but it was not my code.
--
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/2190#discussion_r363397630
5:35 p.m.
miconda commented on this pull request.
+ LM_ERR("no table name\n");
+ return -1;
+ }
+ /* try to add the table */
+ if (add_source(table) != 0) {
+ LM_ERR("could not add table");
+ return -1;
+ }
+
+ gnode = table2dt(table);
+ if (!gnode) {
+ LM_ERR("invalid table '%s'\n", table);
+ return -1;
+ }
+
+ arg = pkg_malloc(sizeof(struct check_blacklist_fs_t));
Maybe the pkg_free() has to be done in the functions you added (the kemi functions),
otherwise the last change you did seems to affect the old code which was not reported to
have issues. It cna be that now it frees for the kamailio.cfg native functions, when it
should not happen.
--
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/2190#discussion_r363399228
5:44 p.m.
@fsantulli pushed 1 commit.
08a17afe1c18ab0f54287b7fb9d22bedad94983d userblacklist: moved pkg_free to kemi functions
instead of native ones.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2190/files/d5294563c3be9bcc7e2157…
5:45 p.m.
fsantulli commented on this pull request.
+ LM_ERR("no table name\n");
+ return -1;
+ }
+ /* try to add the table */
+ if (add_source(table) != 0) {
+ LM_ERR("could not add table");
+ return -1;
+ }
+
+ gnode = table2dt(table);
+ if (!gnode) {
+ LM_ERR("invalid table '%s'\n", table);
+ return -1;
+ }
+
+ arg = pkg_malloc(sizeof(struct check_blacklist_fs_t));
I've done the change as requested. Please check the commit and let me know.
--
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/2190#discussion_r363402966
6:48 p.m.
@fsantulli pushed 1 commit.
5f14c488d9ac416864562f888c40e4f005307c3f userblacklist: fixed rpc_child_init behaviour as
changed add_source
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2190/files/08a17afe1c18ab0f54287b…
7:18 p.m.
@fsantulli pushed 1 commit.
fbc8598ffc4e3c9706c25ac9d5e608c86a2005be userblacklist: added missing pkg_free before
returning
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2190/files/5f14c488d9ac416864562f…
7 Jan
7 Jan
11:33 p.m.
Closed #2190.
--
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/2190#event-2929405030
11:34 p.m.
Reopened #2190.
--
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/2190#event-2929406765
12 Jan
12 Jan
11:58 a.m.
Hi All,
how could i proceed to get the pull request merged ?
Kind regards.
Federico Santulli
Il giorno 8 gen 2020, alle ore 00:34, Federico
Santulli <notifications(a)github.com> ha scritto:
Reopened #2190 <https://github.com/kamailio/kamailio/pull/2190>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/kamailio/kamailio/pull/2190?email_source=notifications&email_token=ABO7UZMYMASRN2K477RSAG3Q4UGQBA5CNFSM4KDHEHIKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOV2NTGLI#event-2929406765>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABO7UZI3DZE7EV65I6JGUDTQ4UGQBANCNFSM4KDHEHIA>.
_______________________________________________
Kamailio (SER) - Development Mailing List
sr-dev(a)lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
13 Jan
13 Jan
6:33 a.m.
No action necessary from your side. :-) I will also have a look later today and merge it
when I did not notice anything else.
--
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/2190#issuecomment-573526469
9:10 p.m.
Pull request merged manually (to adapt the commit logs). This changed code in different
areas , so I decided to not backport it to stable branches right now. After some testing
it could be backported to 5.3 stable branch, though.
--
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/2190#issuecomment-573872121
9:10 p.m.
Closed #2190.
--
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/2190#event-2944776375
1647
days inactive
1654
days old
15 comments
4 participants
participants (4)
-
Daniel-Constantin Mierla -
Federico Santulli
-
Federico Santulli -
Henning Westerholt