- rpc_get function can be also called with only entering the group name information. Before it used to work only with group name and variable name and returning the value for a variable. With the enhancement, it is also possible to retrieve all variable values belonging to a specific group. e.g.: kamcmd cfg.get core. Above given example will print the values of all variables of the configuration group "core".
<!-- 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 --> rpc_get function can be also called with only entering the group name information. Before it used to work only with group name and variable name and returning the value for a variable. With the enhancement, it is also possible to retrieve all variable values belonging to a specific group. e.g.: kamcmd cfg.get core. Above given example will print the values of all variables of the configuration group "core".
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1321
-- Commit Summary --
* cfg_rpc: extending the functionality of cfg.get command
-- File Changes --
M src/modules/cfg_rpc/cfg_rpc.c (112) M src/modules/cfg_rpc/doc/rpc.xml (4)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1321.patch https://github.com/kamailio/kamailio/pull/1321.diff
miconda commented on this pull request.
- str group, var;
+ void *val; + unsigned int val_type; + int ret, n; + unsigned int *group_id; + + n = rpc->scan(c, "SS", &group, &var); + /* 2: both group and variable name are present + * -1: only group is present, print all variables in the group */ + if(n<2) { + if (n == -1) { + var.s = NULL; + var.len = 0; + } + else return; + }
I think that the right way is to read with optional specifier `*`:
``` n = rpc->scan(c, "S*S", &group, &var); ```
The n is two if both were read or 1 if only group is read.
When n is -1, there can be other errors.
Otherwise, the patch is useful, thanks!
Can you do a new patch based on what I suggested? It can be a follow up of this one, we can squash from the web when merging.
hdikme commented on this pull request.
- str group, var;
+ void *val; + unsigned int val_type; + int ret, n; + unsigned int *group_id; + + n = rpc->scan(c, "SS", &group, &var); + /* 2: both group and variable name are present + * -1: only group is present, print all variables in the group */ + if(n<2) { + if (n == -1) { + var.s = NULL; + var.len = 0; + } + else return; + }
Thank you for the review, sure i'll send a new patch with the changes.
@hdikme - planning to have the new patch soon? Just to evaluate if this needs to be in 5.1 or not which is going to be out in few days...
@miconda - I have tested the code with the changes based on your review. First part is indeed more appropriate (S*S) but i have a problem with the value of n. If only one parameter (group name) is given , n is still assigned -1. Either was S*S or SS returns -1. What do you think?
What do you use to send the rpc commands - `kamcmd`? If yes, can you try also with `kamctl rpc cfg.get ...`? I want to see if it is some issue in `ctl` module. For kamctl you need jsonrpcs module to be loaded.
Yes, I use kamcmd as I think this is the new one and this should replace kamctl someday in the future. I tried with kamctl rpc, but it just prints out the help page of kamctl. It seems kamctl does not know rpc as a parameter. Maybe you can provide me a more detailed description how to test this with kamctl? Or maybe it's even faster you test it yourself using kamctl?
Are you testing with master branch (or 5.x)? `kamctl rpc ...` works there.
Btw, kamcmd is not a replacement for kamctl.
kamcmd is only a binrpc client application that connects to ctl module of kamailio.
kamcli might replace kamcmd, but no timeline for that. kamctl is still very active maintained and enhanced with each release.
I have tested it with the version 5.0.1 using kamctl rpc ... and the result hasn't changed. n = rpc->scan(c, "S*S", &group, &var); n equals to -1 in case of using only the group name.
The issue with `n = rpc->scan(c, "S*S", &group, &var);` was due to a bug in `ctl` module, using `kamcmd` revealed that. Using `kamctl` with `jsonrpcs` module was not affected. The bug is fixed now in master and it will be backported.
Besides using the `*` specifier for optional params, your patch in this PR was adjusted so that the `cfg.get` for a group returns a structure with field names and values. All are in master now.
Closed #1321.
-
Daniel-Constantin Mierla -
hdikme