28 Nov
2019
28 Nov
'19
10:24 a.m.
#### 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
- [ ] Small bug fix (non-breaking change which fixes an issue)
- [X] 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
#### Description
Allow pvar as second parameter of avp_subst() function
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2158
-- Commit Summary --
* avpops: avp_subst() support pvar as subst parameter
-- File Changes --
M src/modules/avpops/avpops.c (63)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2158.patch
https://github.com/kamailio/kamailio/pull/2158.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/2158
28 Nov
28 Nov
2:25 p.m.
miconda commented on this pull request.
+ if (get_str_fparam(&subst, msg, fp) != 0)
+ {
+ LM_ERR("error fetching subst re");
+ return -1;
+ }
+
+ LM_DBG("%s preparing %s\n", exports.name, subst.s);
+
+ se=subst_parser(&subst);
+ if (se==0)
+ {
+ LM_ERR("%s: bad subst re %s\n", exports.name, subst.s);
+ return E_BAD_RE;
+ }
+
+ return ops_subst(msg, (struct fis_param**)src, se);
I think that se needs to be freed, because subst_parser() allocates the result.
--
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/2158#pullrequestreview-324438268
5:08 p.m.
linuxmaniac commented on this pull request.
+ if (get_str_fparam(&subst, msg, fp) != 0)
+ {
+ LM_ERR("error fetching subst re");
+ return -1;
+ }
+
+ LM_DBG("%s preparing %s\n", exports.name, subst.s);
+
+ se=subst_parser(&subst);
+ if (se==0)
+ {
+ LM_ERR("%s: bad subst re %s\n", exports.name, subst.s);
+ return E_BAD_RE;
+ }
+
+ return ops_subst(msg, (struct fis_param**)src, se);
Yes, indeed. Thanks!
--
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/2158#discussion_r351934586
5:13 p.m.
@linuxmaniac pushed 1 commit.
889d0aa410c0060441bf1ac3c1ce7c96ac8ed5c0 avpops: avp_subst() support pvar as subst
parameter
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2158/files/51844f0bd8857d5bfbfd17…
2 Dec
2 Dec
7:25 a.m.
Looking further at this patch, it requires to have the second parameter as a single
variable to be considered with "pvar" support, because I see you search for `/`
in the second parameter. It makes it support like:
```
$var(se) = "/abc/xyz/";
avp_subst("$avp(x)", "$var(se)");
```
Right now it support PV in the replacement part of the subst expression, not allowing the
match/regexp part.
Maybe is better to just add a new function that will always evaluate the subst parameter
with variables, allowing more flexible format for subst parameter. At the end, you just
need to have your added code in a dedicated function that evaluates first the parameter
and then compiles the subst expression.
So we can have:
```
avp_susbtx("$avp(x)", "/$fU/$rU/g");
```
Allowing also:
```
$var(se) = "/" + $fU + "/" + $rU + "/";
avp_substx("$avp(x)", "$var(se)");
```
The new function can have a different name than avp_substx(), I used to exemplify here.
--
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/2158#issuecomment-560374420
13 Dec
13 Dec
3:08 a.m.
@linuxmaniac pushed 2 commits.
f611b2073ccb40116cbe2932d6b6b88e441d0134 avpops: add avp_subst_pv()
0fa3c4964466cef1084eca3db9879b4b253fe7a2 avpops: add avp_subst_pv() documentation
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2158/files/889d0aa410c0060441bf1a…
3:12 a.m.
Added ``avp_subst_pv()`` and now this is supported:
```
$(avp(src)[*]) = "testME";
$var(x) = "/e/j/gi";
avp_subst_pv("$avp(src)", "$var(x)");
$var(z) = "j";
$var(y) = "e";
$var(x) = "/" + $var(y) + "/" + $var(z) + "/gi";
avp_subst_pv("$avp(src)", "$var(x)");
avp_subst_pv("$avp(src)", "/$var(y)/$var(z)/gi");
avp_subst_pv("$avp(src)", "/" + $var(y) + "/" + $var(z)
+ "/gi");
avp_subst_pv("$avp(src)", "/e/j/gi");
```
--
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/2158#issuecomment-565344698
19 Dec
19 Dec
4:49 a.m.
Is this is still work in progress or could be merged?
--
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/2158#issuecomment-567417243
4:50 a.m.
I'm waiting for feedback. No more changes from my side.
--
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/2158#issuecomment-567417587
2 Jan
2 Jan
3:44 p.m.
It is a new function now, it can be merged.
--
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/2158#issuecomment-570340302
3 Jan
3 Jan
3:44 a.m.
Merged #2158 into master.
--
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/2158#event-2919663260
1823
days inactive
1859
days old
10 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Henning Westerholt
-
Victor Seva