Remove HAVE_INET_PTON by jorgsowa · Pull Request #13410 · php/php-src

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

Analogical change to #12700

When inet_ntop is available then inet_pton also.

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


 	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

 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

+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

 	/* 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

 				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

 				}
 			}
-#if HAVE_IPV6 && HAVE_INET_PTON
+#if HAVE_IPV6


Suggested change

#if HAVE_IPV6

#ifdef HAVE_IPV6

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

 	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

 	/* 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 bukka, devnexen, dstogov and kocsismate 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

Comment thread configure.ac Outdated

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

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

Conversation

jorgsowa commented Feb 16, 2024

Uh oh!

Uh oh!

petk commented Feb 16, 2024

Uh oh!

jorgsowa commented Feb 16, 2024

Uh oh!

petk commented Feb 17, 2024

Uh oh!

devnexen commented Feb 17, 2024

Uh oh!

jorgsowa commented Feb 17, 2024

Uh oh!

bukka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgsowa commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants