util: fix `-Wsign-conversion` in ip_addr()

[aloisklink]

Explicitly cast unsigned int port to an signed int to supress a -Wsign-conversion warning in uvw::details::ip_addr().

We don't need to worry about too much about overflowing, since libuv internally just converts everything back to an unsigned short later, see https://github.com/libuv/libuv/blob/b3759772d20e89537980d5d43a01ce4c2f2535a8/src/uv-common.c#L257

Original warning

/uvw/src/uvw/util.cpp: In function ‘sockaddr uvw::details::ip_addr(const char*, unsigned int)’:
/uvw/src/uvw/util.cpp:63:47: warning: conversion to ‘int’ from ‘unsigned int’ may change the sign of the result [-Wsign-conversion]
   63 |     if(sockaddr_in addr_in; uv_ip4_addr(addr, port, &addr_in) == 0) {
      |                                               ^~~~
/uvw/src/uvw/util.cpp:65:56: warning: conversion to ‘int’ from ‘unsigned int’ may change the sign of the result [-Wsign-conversion]
   65 |     } else if(sockaddr_in6 addr_in6; uv_ip6_addr(addr, port, &addr_in6) == 0) {
      |                                                        ^~~~

[skypjack]

Maybe it's worth it to change the function signature and make it accept an int value instead? Any concerns?

[aloisklink]

Maybe it's worth it to change the function signature and make it accept an int value instead? Any concerns?

No concerns (other than a potential ABI break)! We could even keep the old function signature by having both a sockaddr ip_addr(const char *addr, int port); and the existing sockaddr ip_addr(const char *addr, unsigned int port); if you don't want to break the ABI:

diff --git a/src/uvw/util.cpp b/src/uvw/util.cpp
index 1e5a696..1fdc025 100644
--- a/src/uvw/util.cpp
+++ b/src/uvw/util.cpp
@@ -60,6 +60,10 @@ UVW_INLINE void common_alloc_callback(uv_handle_t *, std::size_t suggested, uv_b
 }

 UVW_INLINE sockaddr ip_addr(const char *addr, unsigned int port) {
+    return ip_addr(addr, static_cast<int>(port));
+}
+
+UVW_INLINE sockaddr ip_addr(const char *addr, int port) {
     if(sockaddr_in addr_in; uv_ip4_addr(addr, port, &addr_in) == 0) {
         return reinterpret_cast<const sockaddr &>(addr_in);
     } else if(sockaddr_in6 addr_in6; uv_ip6_addr(addr, port, &addr_in6) == 0) {
diff --git a/src/uvw/util.h b/src/uvw/util.h
index 0b549e1..5693eb0 100644
--- a/src/uvw/util.h
+++ b/src/uvw/util.h
@@ -278,6 +278,8 @@ void common_alloc_callback(uv_handle_t *handle, std::size_t suggested, uv_buf_t
     *buf = uv_buf_init(alloc, static_cast<unsigned int>(size));
 }

+sockaddr ip_addr(const char *addr, int port);
+[[deprecated("Please use ip_addr(const char *, int) instead")]]
 sockaddr ip_addr(const char *addr, unsigned int port);
 socket_address sock_addr(const sockaddr_in &addr);
 socket_address sock_addr(const sockaddr_in6 &addr);

But if we're going to break the ABI anyway in PR https://github.com/skypjack/uvw/pull/303, we might as well change the signature in this PR! Let me know which you prefer!

---

To be honest, ports are always an uint16_t anyway, so I don't know why libuv doesn't accept an unsigned short instead of an int. Having an unsigned short will allow the compiler to warn people if their port number would overflow. Maybe I should make a PR to libuv's v2.0.0 branch :shrug:

[skypjack]

Fair enough. Let's keep it as unsigned then. 👍

[aloisklink]

I've made a PR to libuv's v2 branch proposing to switch to an unsigned short for port, see https://github.com/libuv/libuv/pull/4130.

However, even if it gets merged, libuv has been working on their v2 branch since 2014, and they still haven't released a v2, so I've got no idea if there will ever be a v2!