Correct return value of nsapi_dns_query_multiple #5945
Conversation
Documentation states that nsapi_dns_query_multiple returns the number of addresses found on success - it was returning 0. Overloads using SocketAddress are relying on the return value, meaning those calls didn't work at all. Fixes ARMmbed#5921.
kjbracey
force-pushed
the
dns_multiple
branch
from
8029dea to
15a3922
Compare
|
/morph build |
| @@ -243,8 +243,9 @@ static nsapi_size_or_error_t nsapi_dns_query_multiple(NetworkStack *stack, const | |||
| } | |||
|
|
|||
| const uint8_t *response = packet; | |||
| if (dns_scan_response(&response, addr, addr_count) > 0) { | |||
| result = NSAPI_ERROR_OK; | |||
| int count = dns_scan_response(&response, addr, addr_count); | |||
There was a problem hiding this comment.
You might consider changing the return type of dns_scan_response(). It currently returns an int, even though its main return path is return count; and count is an unsigned.
Seems pretty fishy if you ask me. On the other hand, this function returns nsapi_size_or_error_t anyway, and that's an alias to signed int, so the cast from unsigned to int will happen somewhere anyway, so I guess this point is just purely stylistic.
There was a problem hiding this comment.
Yep, we've got to get from addr_count being unsigned ultimately back to that nsapi_size_or_error_t. I don't think it's worth worrying about - any changes are liable to trigger new "comparison of signed and unsigned" warning. Let's keep this change minimal and it can slip into a patch release.
Build : SUCCESSBuild number : 969 Triggering tests/morph test |
Exporter Build : FAILUREBuild number : 656 |
|
/morph export-build |
Test : SUCCESSBuild number : 797 |
Exporter Build : SUCCESSBuild number : 661 |
|
/morph uvisor-test |
Documentation states that
nsapi_dns_query_multiplereturns the number ofaddresses found on success - it was returning 0.
Overloads using
SocketAddressare relying on the return value, meaningthose calls didn't work at all.
Fixes #5921.