Skip to content

nsapi - Standardize support of NSAPI_UNSPEC #2897

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.

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 2 commits into from
Oct 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 15 additions & 2 deletions features/FEATURE_LWIP/lwip-interface/lwip_stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ static bool convert_mbed_addr_to_lwip(ip_addr_t *out, const nsapi_addr_t *in)
#if !LWIP_IPV4
/* For bind() and other purposes, need to accept "null" of other type */
/* (People use IPv4 0.0.0.0 as a general null) */
if (in->version == NSAPI_IPv4 && all_zeros(in->bytes, 4)) {
if (in->version == NSAPI_UNSPEC ||
(in->version == NSAPI_IPv4 && all_zeros(in->bytes, 4))) {
ip_addr_set_zero_ip6(out);
return true;
}
Expand All @@ -135,13 +136,25 @@ static bool convert_mbed_addr_to_lwip(ip_addr_t *out, const nsapi_addr_t *in)
}
#if !LWIP_IPV6
/* For symmetry with above, accept IPv6 :: as a general null */
if (in->version == NSAPI_IPv6 && all_zeros(in->bytes, 16)) {
if (in->version == NSAPI_UNSPEC ||
(in->version == NSAPI_IPv6 && all_zeros(in->bytes, 16))) {
ip_addr_set_zero_ip4(out);
return true;
}
#endif
#endif

#if LWIP_IPV4 && LWIP_IPV6
Copy link
Contributor

@kjbracey kjbracey Oct 5, 2016

Choose a reason for hiding this comment

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

Not sure about this flopping towards IPV4. Could it at least follow the IPv4/IPv6 preference? Ideally we'd convert to lwip's own IPADDR_TYPE_ANY, but that isn't fully supported in their ip_addr_t either - it's used internally, but I don't believe it's valid for external users.

Jesus, this ifdeffing is confusing. Not sure we can do better though.

if (in->version == NSAPI_UNSPEC) {
#if IP_VERSION_PREF == PREF_IPV4
ip_addr_set_zero_ip4(out);
#else
ip_addr_set_zero_ip6(out);
#endif
return true;
}
#endif

return false;
}

Expand Down
5 changes: 0 additions & 5 deletions features/netsocket/NetworkInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ int NetworkInterface::set_dhcp(bool dhcp)
}

// DNS operations go through the underlying stack by default
int NetworkInterface::gethostbyname(const char *name, SocketAddress *address)
{
return get_stack()->gethostbyname(name, address);
}

int NetworkInterface::gethostbyname(const char *name, SocketAddress *address, nsapi_version_t version)
{
return get_stack()->gethostbyname(name, address, version);
Expand Down
19 changes: 3 additions & 16 deletions features/netsocket/NetworkInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,6 @@ class NetworkInterface {
*/
virtual int disconnect() = 0;

/** Translates a hostname to an IP address
*
* The hostname may be either a domain name or an IP address. If the
* hostname is an IP address, no network transactions will be performed.
*
* If no stack-specific DNS resolution is provided, the hostname
* will be resolve using a UDP socket on the stack.
*
* @param address Destination for the host SocketAddress
* @param host Hostname to resolve
* @return 0 on success, negative error code on failure
*/
virtual int gethostbyname(const char *host, SocketAddress *address);

/** Translates a hostname to an IP address with specific version
*
* The hostname may be either a domain name or an IP address. If the
Expand All @@ -124,10 +110,11 @@ class NetworkInterface {
*
* @param address Destination for the host SocketAddress
* @param host Hostname to resolve
* @param version IP version of address to resolve
* @param version IP version of address to resolve, NSAPI_UNSPEC indicates
* version is chosen by the stack (defaults to NSAPI_UNSPEC)
* @return 0 on success, negative error code on failure
*/
virtual int gethostbyname(const char *host, SocketAddress *address, nsapi_version_t version);
virtual int gethostbyname(const char *host, SocketAddress *address, nsapi_version_t version = NSAPI_UNSPEC);

/** Add a domain name server to list of servers to query
*
Expand Down
24 changes: 1 addition & 23 deletions features/netsocket/NetworkStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,6 @@


// Default NetworkStack operations
int NetworkStack::gethostbyname(const char *name, SocketAddress *address)
{
// check for simple ip addresses
if (address->set_ip_address(name)) {
return 0;
}

return nsapi_dns_query(this, name, address);
}

int NetworkStack::gethostbyname(const char *name, SocketAddress *address, nsapi_version_t version)
{
// check for simple ip addresses
Expand Down Expand Up @@ -100,25 +90,13 @@ class NetworkStackWrapper : public NetworkStack
return address->get_ip_address();
}

virtual int gethostbyname(const char *name, SocketAddress *address)
{
if (!_stack_api()->gethostbyname) {
return NetworkStack::gethostbyname(name, address);
}

nsapi_addr_t addr = {NSAPI_IPv4, 0};
int err = _stack_api()->gethostbyname(_stack(), name, &addr, NSAPI_UNSPEC);
address->set_addr(addr);
return err;
}

virtual int gethostbyname(const char *name, SocketAddress *address, nsapi_version_t version)
{
if (!_stack_api()->gethostbyname) {
return NetworkStack::gethostbyname(name, address, version);
}

nsapi_addr_t addr = {NSAPI_IPv4, 0};
nsapi_addr_t addr = {NSAPI_UNSPEC, 0};
int err = _stack_api()->gethostbyname(_stack(), name, &addr, version);
address->set_addr(addr);
return err;
Expand Down
19 changes: 3 additions & 16 deletions features/netsocket/NetworkStack.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,6 @@ class NetworkStack
*/
virtual const char *get_ip_address() = 0;

/** Translates a hostname to an IP address
*
* The hostname may be either a domain name or an IP address. If the
* hostname is an IP address, no network transactions will be performed.
*
* If no stack-specific DNS resolution is provided, the hostname
* will be resolve using a UDP socket on the stack.
*
* @param host Hostname to resolve
* @param address Destination for the host SocketAddress
* @return 0 on success, negative error code on failure
*/
virtual int gethostbyname(const char *host, SocketAddress *address);

/** Translates a hostname to an IP address with specific version
*
* The hostname may be either a domain name or an IP address. If the
Expand All @@ -65,10 +51,11 @@ class NetworkStack
*
* @param host Hostname to resolve
* @param address Destination for the host SocketAddress
* @param version IP version of address to resolve
* @param version IP version of address to resolve, NSAPI_UNSPEC indicates
* version is chosen by the stack (defaults to NSAPI_UNSPEC)
* @return 0 on success, negative error code on failure
*/
virtual int gethostbyname(const char *host, SocketAddress *address, nsapi_version_t version);
virtual int gethostbyname(const char *host, SocketAddress *address, nsapi_version_t version = NSAPI_UNSPEC);

/** Add a domain name server to list of servers to query
*
Expand Down
52 changes: 29 additions & 23 deletions features/netsocket/SocketAddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,19 @@ void SocketAddress::set_port(uint16_t port)

const char *SocketAddress::get_ip_address() const
{
char *ip_address = (char *)_ip_address;
if (_addr.version == NSAPI_UNSPEC) {
Copy link
Contributor

@kjbracey kjbracey Oct 5, 2016

Choose a reason for hiding this comment

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

So UNSPEC SocketAddress round-trips via NULL string pointer, if I read correctly. Seems fine. But would still vaguely prefer a real string, so users don't have to take care when doing printf("%s", foo->get_ip_address()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does have the benefit of matching the behaviour of iface->get_ip_address() when not connected.

return NULL;
}

if (!ip_address[0]) {
if (!_ip_address[0]) {
if (_addr.version == NSAPI_IPv4) {
ipv4_to_address(ip_address, _addr.bytes);
ipv4_to_address(_ip_address, _addr.bytes);
} else if (_addr.version == NSAPI_IPv6) {
ipv6_to_address(ip_address, _addr.bytes);
ipv6_to_address(_ip_address, _addr.bytes);
}
}

return ip_address;
return _ip_address;
}

const void *SocketAddress::get_ip_bytes() const
Expand All @@ -250,34 +252,38 @@ uint16_t SocketAddress::get_port() const

SocketAddress::operator bool() const
{
int count = 0;
if (_addr.version == NSAPI_IPv4) {
count = NSAPI_IPv4_BYTES;
} else if (_addr.version == NSAPI_IPv6) {
count = NSAPI_IPv6_BYTES;
}
for (int i = 0; i < NSAPI_IPv4_BYTES; i++) {
if (_addr.bytes[i]) {
return true;
}
}

for (int i = 0; i < count; i++) {
if (_addr.bytes[i]) {
return true;
return false;
} else if (_addr.version == NSAPI_IPv6) {
for (int i = 0; i < NSAPI_IPv6_BYTES; i++) {
if (_addr.bytes[i]) {
return true;
}
}
}

return false;
return false;
} else {
return false;
}
}

bool operator==(const SocketAddress &a, const SocketAddress &b)
{
int count = 0;
if (a._addr.version == NSAPI_IPv4 && b._addr.version == NSAPI_IPv4) {
count = NSAPI_IPv4_BYTES;
} else if (a._addr.version == NSAPI_IPv6 && b._addr.version == NSAPI_IPv6) {
count = NSAPI_IPv6_BYTES;
} else {
if (!a && !b) {
return true;
} else if (a._addr.version != b._addr.version) {
return false;
} else if (a._addr.version == NSAPI_IPv4) {
return memcmp(a._addr.bytes, b._addr.bytes, NSAPI_IPv4_BYTES) == 0;
} else if (a._addr.version == NSAPI_IPv6) {
return memcmp(a._addr.bytes, b._addr.bytes, NSAPI_IPv6_BYTES) == 0;
}

return (memcmp(a._addr.bytes, b._addr.bytes, count) == 0);
}

bool operator!=(const SocketAddress &a, const SocketAddress &b)
Expand Down
2 changes: 1 addition & 1 deletion features/netsocket/SocketAddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class SocketAddress {
private:
void _SocketAddress(NetworkStack *iface, const char *host, uint16_t port);

char _ip_address[NSAPI_IP_SIZE];
mutable char _ip_address[NSAPI_IP_SIZE];
nsapi_addr_t _addr;
uint16_t _port;
};
Expand Down
2 changes: 1 addition & 1 deletion features/netsocket/nsapi_dns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static void dns_append_question(uint8_t **p, const char *host, nsapi_version_t v
dns_append_byte(p, 0);

// fill out question footer
if (version == NSAPI_IPv4) {
if (version != NSAPI_IPv6) {
dns_append_word(p, RR_A); // qtype = ipv4
} else {
dns_append_word(p, RR_AAAA); // qtype = ipv6
Expand Down
10 changes: 6 additions & 4 deletions features/netsocket/nsapi_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,18 @@ typedef enum nsapi_error {
* @enum nsapi_version_t
*/
typedef enum nsapi_version {
NSAPI_IPv4, /*!< Address is IPv4 */
NSAPI_IPv6, /*!< Address is IPv6 */
NSAPI_UNSPEC /*!< Address is unspecified */
NSAPI_UNSPEC, /*!< Address is unspecified */
NSAPI_IPv4, /*!< Address is IPv4 */
NSAPI_IPv6, /*!< Address is IPv6 */
} nsapi_version_t;

/** IP address structure for passing IP addresses by value
*/
typedef struct nsapi_addr {
/** IP version
* NSAPI_IPv4 or NSAPI_IPv6 (NSAPI_UNSPEC not currently supported)
* - NSAPI_IPv4
* - NSAPI_IPv6
* - NSAPI_UNSPEC
*/
nsapi_version_t version;

Expand Down