Remove HAVE_INET_PTON #13410

jorgsowa · 2024-02-16T20:26:19Z

Analogical change to #12700

When inet_ntop is available then inet_pton also.

configure.ac

petk · 2024-02-16T22:44:10Z

Thanks @jorgsowa, I think this is logical indeed, if inet_pton is universally available on the current systems. Then probably also these tests can be simplified in the SKIPIF sections:

ext/standard/tests/network/inet.phpt
ext/standard/tests/network/inet_ipv6.phpt

jorgsowa · 2024-02-16T23:27:35Z

@petk Thanks for review. Tests adjusted

petk · 2024-02-17T06:10:48Z

Perfect, thanks. I'm not sure if this would be too much for this PR, but probably also the inet_aton can be now completely made obsolete and can be removed together with the Windows implementation file(s). The inet_aton is deprecated anyway. If I'm not mistaken...

devnexen · 2024-02-17T08:37:03Z

Perfect, thanks. I'm not sure if this would be too much for this PR, but probably also the inet_aton can be now completely made obsolete and can be removed together with the Windows implementation file(s). The inet_aton is deprecated anyway. If I'm not mistaken...

I m not objecting neither. If the OP does not want to go any further, it looks good already.

jorgsowa · 2024-02-17T10:17:58Z

I'm not sure if this would be too much for this PR, but probably also the inet_aton can be now completely made obsolete and can be removed together with the Windows implementation file(s). The inet_aton is deprecated anyway. If I'm not mistaken...

No problem to me. I will cover it in the next PR. 🙂

bukka

Looks good. Just minor NITs

bukka · 2024-02-17T13:12:03Z

ext/standard/dns.c

@@ -157,7 +157,7 @@ PHP_FUNCTION(gethostbyaddr)
 	hostname = php_gethostbyaddr(addr);

 	if (hostname == NULL) {
-#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON)
+#if defined(HAVE_IPV6)


Suggested change

#if defined(HAVE_IPV6)

#ifdef HAVE_IPV6

bukka · 2024-02-17T13:12:15Z

ext/standard/dns.c

@@ -172,7 +172,7 @@ PHP_FUNCTION(gethostbyaddr)
 /* {{{ php_gethostbyaddr */
 static zend_string *php_gethostbyaddr(char *ip)
 {
-#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON)
+#if defined(HAVE_IPV6)


Suggested change

#if defined(HAVE_IPV6)

#ifdef HAVE_IPV6

bukka · 2024-02-17T13:13:01Z

ext/standard/tests/network/ip2long_variation2.phpt

+if(PHP_INT_SIZE != 4) { die('skip 32 bit only'); }
+if(strtolower(substr(PHP_OS, 0, 3)) == 'aix') { die('skip not for AIX'); }


NIT: space after if

bukka · 2024-02-17T13:13:57Z

main/network.c

@@ -545,7 +541,7 @@ PHPAPI int php_network_parse_network_address_with_port(const char *addr, zend_lo

 	/* first, try interpreting the address as a numeric address */

-#if HAVE_IPV6 && HAVE_INET_PTON
+#if HAVE_IPV6


Suggested change

#if HAVE_IPV6

#ifdef HAVE_IPV6

think it should always test definition

Agreed, I have a long-standing (very outdated PR (#5526) now) to try to enable -Wundef to check these sorts of issues at compile time.

Do you need help with this PR, @Girgias ?

You can take it over, I was hitting some issue with YACC generating something that I couldn't check.

bukka · 2024-02-17T13:14:10Z

main/network.c

@@ -852,25 +844,21 @@ php_socket_t php_network_connect_socket_to_host(const char *host, unsigned short
 			union {
 				struct sockaddr common;
 				struct sockaddr_in in4;
-#if HAVE_IPV6 && HAVE_INET_PTON
+#if HAVE_IPV6


Suggested change

#if HAVE_IPV6

#ifdef HAVE_IPV6

bukka · 2024-02-17T13:14:22Z

main/network.c

 					local_address_len = sizeof(struct sockaddr_in);
 					local_address.in4.sin_family = sa->sa_family;
 					local_address.in4.sin_port = htons(bindport);
 					memset(&(local_address.in4.sin_zero), 0, sizeof(local_address.in4.sin_zero));
 				}
 			}
-#if HAVE_IPV6 && HAVE_INET_PTON
+#if HAVE_IPV6


Suggested change

#if HAVE_IPV6

#ifdef HAVE_IPV6

Girgias · 2024-02-17T13:46:31Z

ext/openssl/xp_ssl.c

@@ -130,7 +130,7 @@
 #define PHP_X509_NAME_ENTRY_TO_UTF8(ne, i, out) \
 	ASN1_STRING_to_UTF8(&out, X509_NAME_ENTRY_get_data(X509_NAME_get_entry(ne, i)))

-#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON)
+#if defined(HAVE_IPV6)


Suggested change

#if defined(HAVE_IPV6)

#ifdef HAVE_IPV6

Girgias · 2024-02-17T13:48:29Z

main/network.c

@@ -545,7 +541,7 @@ PHPAPI int php_network_parse_network_address_with_port(const char *addr, zend_lo

 	/* first, try interpreting the address as a numeric address */

-#if HAVE_IPV6 && HAVE_INET_PTON
+#if HAVE_IPV6


Agreed, I have a long-standing (very outdated PR (#5526) now) to try to enable -Wundef to check these sorts of issues at compile time.

jorgsowa · 2024-02-20T21:36:52Z

Thank you all for the reviews. If I should add something else then please let me know. Otherwise, I don't know who should I ask for approval and merge now.

jorgsowa requested review from kocsismate, dstogov, bukka and devnexen as code owners February 16, 2024 20:26

github-actions bot added Category: Build System Extension: openssl Extension: sockets Extension: standard SAPI: litespeed Category: Optimizer labels Feb 16, 2024

jorgsowa added 5 commits February 16, 2024 21:30

Remove HAVE_INET_PTON

4d7b5e8

Remove inet_pton from configure

b921236

Generated stub

9eda2cd

Reverted skip of test on Darwin

ad6a418

Syntax changes

e7e7a25

jorgsowa force-pushed the remove_HAVE_INET_PTON branch from fe53851 to e7e7a25 Compare February 16, 2024 20:31

Generate stub

add1f37

petk reviewed Feb 16, 2024

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

jorgsowa added 2 commits February 17, 2024 00:26

Inlined function checks

02a97cf

Removed skips for functions inet_ntop and inet_pton

0778638

bukka reviewed Feb 17, 2024

View reviewed changes

Girgias reviewed Feb 17, 2024

View reviewed changes

jorgsowa added 2 commits February 17, 2024 21:36

Used #ifdef HAVE_IPV6 instead of #if

dc43dd1

Improved formatting of php in test

6eab8e6

github-actions bot added the Extension: ftp label Feb 17, 2024

Reverted indentation in network.c

bf1fad7

Girgias approved these changes Feb 21, 2024

View reviewed changes

Girgias merged commit e630aac into php:master Feb 21, 2024

jorgsowa mentioned this pull request Feb 22, 2024

Remove inet_aton function #13479

Closed

jorgsowa deleted the remove_HAVE_INET_PTON branch August 7, 2024 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove HAVE_INET_PTON #13410

Remove HAVE_INET_PTON #13410

jorgsowa commented Feb 16, 2024

petk commented Feb 16, 2024

jorgsowa commented Feb 16, 2024

petk commented Feb 17, 2024

devnexen commented Feb 17, 2024

jorgsowa commented Feb 17, 2024

bukka left a comment

bukka Feb 17, 2024

bukka Feb 17, 2024

bukka Feb 17, 2024

bukka Feb 17, 2024

Girgias Feb 17, 2024

jorgsowa Feb 17, 2024

Girgias Feb 18, 2024

bukka Feb 17, 2024

bukka Feb 17, 2024

Girgias Feb 17, 2024

Girgias Feb 17, 2024

jorgsowa commented Feb 20, 2024

		if(PHP_INT_SIZE != 4) { die('skip 32 bit only'); }
		if(strtolower(substr(PHP_OS, 0, 3)) == 'aix') { die('skip not for AIX'); }

Remove HAVE_INET_PTON #13410

Remove HAVE_INET_PTON #13410

Conversation

jorgsowa commented Feb 16, 2024

petk commented Feb 16, 2024

jorgsowa commented Feb 16, 2024

petk commented Feb 17, 2024

devnexen commented Feb 17, 2024

jorgsowa commented Feb 17, 2024

bukka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgsowa commented Feb 20, 2024