Add support to new Android DNS functions android_res_nquery android_res_nresult (API 29+) #4333
Conversation
Add support android_res_nquery for DNS (Android API 29+)
|
|
|
Need to implement the check on Android API Level too |
Use dlopen libandroid.so then dlsym to load functions android_res_nquery, android_res_nresult and android_res_cancel. This avoids devices with Android API < 29 to fail to load the library due to absence of the 29+ functions, and let them fallback directly to call the default getaddrinfo()
|
I found it's trickier than I thought to "hide" the API 29+ functions from devices with API < 29. So, I dynamically loaded the functions android_res_nquery, android_res_nresult and android_res_cancel and did this only in case of API >= 29 I tested it on : |
pjlib/src/pj/addr_resolv_sock.c
Outdated
| #include <sys/system_properties.h> // For getting Android Device API Version | ||
| #include <dlfcn.h> | ||
| #endif | ||
|
|
There was a problem hiding this comment.
use #elif here, to make it obvious that only one condition can be satisfied
also indent the #include.
There was a problem hiding this comment.
Thanks for your review.
I put the statement below PJ_GETADDRINFO_USE_CFHOST and changed it to #elif.
I also indented the #include
pjlib/src/pj/addr_resolv_sock.c
Outdated
| if (atoi(sdk_value) >= 29) { | ||
| void *libandroid_handle = dlopen("libandroid.so", RTLD_NOW); | ||
| if (libandroid_handle) { | ||
| android_res_nquery_ptr = (android_res_nquery_fn) dlsym(libandroid_handle, "android_res_nquery"); |
There was a problem hiding this comment.
minor: all col needs to be <= 80
There was a problem hiding this comment.
I reviewed all the lines where columns were over 80 and added linSorry, I've just learnt this for C. Thank you.
pjlib/src/pj/addr_resolv_sock.c
Outdated
| struct pollfd fds; | ||
| fds.fd = resp_handle; | ||
| fds.events = POLLIN; | ||
| int ret = poll(&fds, 1, 5000); // 5-second timeout |
There was a problem hiding this comment.
create a compile-time macro for the timeout (under all the #include above), so user can easily modify it should they wish to.
There was a problem hiding this comment.
I added a macro ANDROID_DNS_TIMEOUT_MS for this.
Also I created the macro THIS_FILE as in other source files, for logging usage.
| */ | ||
| res->ai_socktype != 0) | ||
| { | ||
| continue; |
pjlib/src/pj/addr_resolv_sock.c
Outdated
| android_res_cancel_ptr(resp_handle); | ||
| return PJ_ERESOLVE; | ||
| } | ||
| // Step 3: Get DNS response |
There was a problem hiding this comment.
Use /* */ for comment.
https://docs.pjsip.org/en/latest/get-started/coding-style.html#multiline-comment-indentation
There was a problem hiding this comment.
I corrected the comments so that they match the requirement.
| char sdk_value[8] = ""; | ||
| __system_property_get("ro.build.version.sdk", sdk_value); | ||
| if (atoi(sdk_value) >= 29) { | ||
| void *libandroid_handle = dlopen("libandroid.so", RTLD_NOW); |
There was a problem hiding this comment.
Why it is loaded manually? If modifying build config, e.g: adding library or LD_FLAGS, is needed, perhaps we can help.
Update: ah sorry, just read the discussion above, I guess it is better to also put comments about this in the code.
There was a problem hiding this comment.
Thanks for your review. I corrected it in the last commit and added a comment over that block to explain the reason why I used function pointers to load API 29+ functions only when Android API Level verification is done.
Format for max 80 cols transform the PJ_GETADDRINFO_USE_ANDROID to elif indentation correction add comments for dynamic function loading
Add support to new Android DNS functions android_res_nquery android_res_nresult (API 29+) in pjlib/src/pj/addr_resolv_sock.c
New macro
#define PJ_GETADDRINFO_USE_ANDROID 1I looked at the Android documentation to enhance default getaddrinfo (very slow in recent Android APIs), and found that a native method for DNS resolution is available starting Android API 29, based on android_res_nquery and android_res_nresult functions.
I implemented it and it's definitely faster than default getaddrinfo, the delay between clicking on "Start Library" and actual TLS socket establishment is next to nothing (110 ms)