[kamailio/kamailio] core/resolve: Check dns_cache_init and choose appropriate functions (PR #3858)
<!-- 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 - [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 - [x] Related to issue #3350
#### Description <!-- Describe your changes in detail --> This PR fixes a crash when `dns_cache_init=off`.
The issue is described in #3350 and the crash is caused when trying to search in `dns_cache.c` [LOCK_DNS_HASH();](https://github.com/kamailio/kamailio/blob/d4629be286fc6d3cf61574e34ee877b2c5...).
When `dns_cache_init=off`, the lock is never initialized and when trying to lock it crashes.
Per my understanding, when `dns_cache_init=off`, no calls to `dns_cache.c` should be made and instead call the alternative ones from `resolve.c`, since the whole functions are compiled only when USE_DNS_CACHE definition is present.
Just a question to clarify, what is the reasoning behind USE_DNS_CACHE definition and a separate `dns_cache_init` parameter? You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3858
-- Commit Summary --
* core/resolve: Check dns_cache_init and choose appropriate functions
-- File Changes --
M src/core/resolve.c (16)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3858.patch https://github.com/kamailio/kamailio/pull/3858.diff
@xkaraman pushed 1 commit.
df02022cb9f02b809be4069b596c26485b2f23cb core/resolve: Check dns_cache_init and choose appropriate functions
@xkaraman pushed 1 commit.
1babeb1941b8b068144b4ef988c0ad31e813e68f core/resolve: Check dns_cache_init and choose appropriate functions
@dilyanpalauzov commented on this pull request.
@@ -1623,7 +1623,12 @@ struct hostent *no_naptr_srv_sip_resolvehost(
srv_name.s = tmp_srv; srv_name.len = strlen(tmp_srv); #ifdef USE_DNS_CACHE - he = dns_srv_get_he(&srv_name, port, dns_flags); + if(dns_cache_init) { + he = dns_srv_get_he(&srv_name, port, dns_flags); + } else { + LM_WARN("USE_DNS_CACHE is defined but dns_cache_init=off\n");
Why is this a run-time warning, depending on how Kamailio is compiled and current settings. I mean the distributions compile Kamailio and let the users select, `dns_cache_init` ON or OFF. With this change users will get a warning, they do not to fix, as it will be up to the distribution to fix the warning, by preventing the users to toggle `dns_cache_init`.
@dilyanpalauzov commented on this pull request.
@@ -1833,6 +1847,23 @@ ip_addr_t *str2ip(str *st)
return ipb; }
+struct hostent *__resolvehost(char *name) +{ + if(dns_cache_init) { + return dns_resolvehost(name); + } else { + return _resolvehost(name); + } +} + +struct hostent *__sip_resolvehost(str *name, unsigned short *port, char *proto) +{ + if(dns_cache_init) { + return dns_sip_resolvehost(name, port, proto); + } else {
This `else` can be skipped: ```c if (x) return y; /*skipped else*/ return z; ```
@xkaraman pushed 1 commit.
ce47096d86bdf9379091d0a48676763d9b249a83 core/resolve: Check dns_cache_init and choose appropriate functions
@xkaraman commented on this pull request.
@@ -1623,7 +1623,12 @@ struct hostent *no_naptr_srv_sip_resolvehost(
srv_name.s = tmp_srv; srv_name.len = strlen(tmp_srv); #ifdef USE_DNS_CACHE - he = dns_srv_get_he(&srv_name, port, dns_flags); + if(dns_cache_init) { + he = dns_srv_get_he(&srv_name, port, dns_flags); + } else { + LM_WARN("USE_DNS_CACHE is defined but dns_cache_init=off\n");
You are right, removed.
@xkaraman commented on this pull request.
@@ -1833,6 +1847,23 @@ ip_addr_t *str2ip(str *st)
return ipb; }
+struct hostent *__resolvehost(char *name) +{ + if(dns_cache_init) { + return dns_resolvehost(name); + } else { + return _resolvehost(name); + } +} + +struct hostent *__sip_resolvehost(str *name, unsigned short *port, char *proto) +{ + if(dns_cache_init) { + return dns_sip_resolvehost(name, port, proto); + } else {
True, both way are valid though. I think it's clearer with the `else`, but if maintainers have a preference I am fine to change that.
@xkaraman pushed 1 commit.
0275d334c5b25548e92c40bb981540e2ec61d986 core/resolve: Check dns_cache_init and choose appropriate functions
@xkaraman: fine to merge it from my point of view, thanks!
Thanks, merging!
Merged #3858 into master.
-
Daniel-Constantin Mierla -
Xenofon Karamanos
-
Дилян Палаузов