Skip to content

Correct return value of nsapi_dns_query_multiple #5945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Correct return value of nsapi_dns_query_multiple #5945

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions features/netsocket/nsapi_dns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ static nsapi_size_or_error_t nsapi_dns_query_multiple(NetworkStack *stack, const
socket.set_timeout(DNS_TIMEOUT);

// create network packet
uint8_t *packet = (uint8_t *)malloc(DNS_BUFFER_SIZE);
uint8_t * const packet = (uint8_t *)malloc(DNS_BUFFER_SIZE);
if (!packet) {
return NSAPI_ERROR_NO_MEMORY;
}
Expand Down Expand Up @@ -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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (count > 0) {
result = count;
}

/* The DNS response is final, no need to check other servers */
Expand Down