missing header for inet_ntoa

[remicollet]

To avoid

In file included from /opt/remi/php80/root/usr/include/php/main/php.h:35,
                 from /dev/shm/BUILD/php80-php-pecl-memcached-3.1.5/NTS/php_memcached.h:20,
                 from /dev/shm/BUILD/php80-php-pecl-memcached-3.1.5/NTS/php_memcached_server.c:17:
/dev/shm/BUILD/php80-php-pecl-memcached-3.1.5/NTS/php_memcached_server.c: In function 's_handle_memcached_event':
/dev/shm/BUILD/php80-php-pecl-memcached-3.1.5/NTS/php_memcached_server.c:592:29: warning: implicit declaration of function 'inet_ntoa' [-Wimplicit-function-declaration]
  592 |     ZVAL_STRING(&zremoteip, inet_ntoa (addr_in.sin_addr));
      |                             ^~~~~~~~~
/opt/remi/php80/root/usr/include/php/Zend/zend_API.h:657:21: note: in definition of macro 'ZVAL_STRING'
  657 |   const char *_s = (s);     \
      |                     ^
/opt/remi/php80/root/usr/include/php/Zend/zend_API.h:657:20: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  657 |   const char *_s = (s);     \
      |                    ^
/dev/shm/BUILD/php80-php-pecl-memcached-3.1.5/NTS/php_memcached_server.c:592:5: note: in expansion of macro 'ZVAL_STRING'
  592 |     ZVAL_STRING(&zremoteip, inet_ntoa (addr_in.sin_addr));
   
```   |     ^~~~~~~~~~~

[remicollet]

This is optional as already done in php, but seems more robust (PHP can change)

[remicollet]

@sodabrew @m6w6 please review

[sodabrew]

PHP source code includes the specific heater where inet_ functions are used, e.g.
https://github.com/php/php-src/blob/3e01f5afb1b52fe26a956190296de0192eedeec1/ext/sockets/sockets.c#L47

[sodabrew]

lgtm

[remicollet]

Additional notes: inet_ntoa seems bad... only manage IPV4.... will be better to switch to inet_ntop or any other solution compatible with IPv6 (but out of the scope of this PR)

[m6w6]

arpa/inet.h seems to only be missing on windows, so either check should be sufficient. 👍

[remicollet]

I think we can do something like:

diff --git a/php_memcached_server.c b/php_memcached_server.c
index 83bd15e..a266263 100644
--- a/php_memcached_server.c
+++ b/php_memcached_server.c
@@ -20,6 +20,7 @@
 #if HAVE_ARPA_INET_H
 #include <arpa/inet.h>
 #endif
+#include <main/php_network.h>
 
 #include <event2/listener.h>
 
@@ -588,12 +589,24 @@ void s_handle_memcached_event (evutil_socket_t fd, short what, void *arg)
 			zval params[2];
 			protocol_binary_response_status retval;
 
-			struct sockaddr_in addr_in;
-			socklen_t addr_in_len = sizeof(addr_in);
-
-			if (getpeername (fd, (struct sockaddr *) &addr_in, &addr_in_len) == 0) {
-				ZVAL_STRING(&zremoteip, inet_ntoa (addr_in.sin_addr));
-				ZVAL_LONG(&zremoteport, ntohs (addr_in.sin_port));
+			php_sockaddr_storage sa;
+			struct sockaddr *addr = (struct sockaddr *)&sa;
+			socklen_t addr_in_len = sizeof(sa);
+
+			if (getpeername (fd, addr, &addr_in_len) == 0) {
+				if (addr->sa_family == AF_INET) {
+					struct sockaddr_in *sa = (struct sockaddr_in *)addr;
+					ZVAL_STRING(&zremoteip, inet_ntoa(sa->sin_addr));
+					ZVAL_LONG(&zremoteport, ntohs (sa->sin_port));
+				}
+#if HAVE_IPV6 && HAVE_INET_NTOP
+				if (addr->sa_family == AF_INET6) {
+					char buf[INET6_ADDRSTRLEN];
+					struct sockaddr_in6 *sa = (struct sockaddr_in6 *)addr;
+					ZVAL_STRING(&zremoteip, inet_ntop(AF_INET6, &sa->sin6_addr, buf, INET6_ADDRSTRLEN));
+					ZVAL_LONG(&zremoteport, ntohs (sa->sin6_port));
+				}
+#endif
 			} else {
 				php_error_docref(NULL, E_WARNING, "getpeername failed: %s", strerror (errno));
 				ZVAL_NULL(&zremoteip);

BTW a much more simpler approach will be to use PHP API and php_network_populate_name_from_sockaddr but as this return a single string (1.2.3.4:123 or [11:22:33:44]:123 or /path/to/local.sock) so will required a change to callback args...

[m6w6]

Hold on 😄 I have a PR in the works, actually making MemcachedServer usable.

[m6w6]

Heh, that actually looks even better, gonna adapt your safe version.

Re: php_network_populate_name_from_sockaddr, yes, I looked at that and had the same concerns.

[m6w6]

Given that this API is not documented and was unusable until now, I'll go with php_network_populate_name_from_sockaddr.

[remicollet]

Given that this API is not documented and was unusable until now,

Yes, I noticed that....

I'll go with php_network_populate_name_from_sockaddr.

Great +1

Will close this one when your will be ready.

[remicollet]

#474 really better